Skip to content

Log a warning if there are download duplicates for MAST#2510

Merged
bsipocz merged 1 commit into
astropy:mainfrom
jdavies-st:mast-log-duplicate-warning
Sep 8, 2022
Merged

Log a warning if there are download duplicates for MAST#2510
bsipocz merged 1 commit into
astropy:mainfrom
jdavies-st:mast-log-duplicate-warning

Conversation

@jdavies-st

Copy link
Copy Markdown
Contributor

This is a follow-up PR in response to #2497 (comment)

Instead of issuing a warnings.warn when duplicate downloads are culled, this PR uses log.warning. I've removed the warning class I'd created and instead just logged the warning. Test is updated as well.

@codecov

codecov Bot commented Sep 7, 2022

Copy link
Copy Markdown

Codecov Report

Merging #2510 (b348489) into main (3a57b4c) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2510      +/-   ##
==========================================
+ Coverage   62.97%   62.98%   +0.01%     
==========================================
  Files         133      133              
  Lines       17308    17308              
==========================================
+ Hits        10900    10902       +2     
+ Misses       6408     6406       -2     
Impacted Files Coverage Δ
astroquery/exceptions.py 0.00% <ø> (ø)
astroquery/mast/observations.py 76.66% <0.00%> (ø)
astroquery/svo_fps/core.py 95.45% <0.00%> (+0.21%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bsipocz bsipocz added the mast label Sep 7, 2022
Comment thread astroquery/mast/observations.py Outdated
if number_unique < number:
warnings.warn(f"{number - number_unique} of {number} products were duplicates."
f"Only downloading {number_unique} unique product(s).", DuplicateResultsWarning)
log.warning(f"{number - number_unique} of {number} products were duplicates. "

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels more like an info level message, but we don't really have a clear policy on the boundaries, so you can keep it as a warning if you prefer.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. I will change it.

@jdavies-st jdavies-st force-pushed the mast-log-duplicate-warning branch from be56800 to f6ab293 Compare September 8, 2022 07:27
@jdavies-st jdavies-st force-pushed the mast-log-duplicate-warning branch from f6ab293 to b348489 Compare September 8, 2022 07:37
@bsipocz

bsipocz commented Sep 8, 2022

Copy link
Copy Markdown
Member

Thanks @jdavies-st!

@bsipocz bsipocz merged commit c9972e0 into astropy:main Sep 8, 2022
@bsipocz bsipocz added this to the v0.4.7 milestone Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants