Deal with warnings #361
Deal with warnings #361
Conversation
|
So far I've just added comments (including the full warning message) in the code where they're raised. I'd like feedback on how to deal with them.
|
| @@ -336,6 +340,9 @@ def HST_get_spec(sample_table, search_radius_arcsec, datadir, verbose, | |||
| continue | |||
|
|
|||
| # Download | |||
| # Observations.download_products prints an info message for every download, like: | |||
There was a problem hiding this comment.
There should be a kwarg in astroquery.mast to shut these up, if it's missing then please open an issue and we'll get it fixed.
There was a problem hiding this comment.
I don't see a kwarg, so I'll open an issue.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ok, there's a kwarg verbose=False that will do this, but we need the latest "dev" version of astroquery (0.4.8.dev9474) to get it. Fornax has 0.4.7 which is the latest version available through pip unless you add the --pre flag. @bsipocz have we figured out how to handle these "dev" dependencies in requirements.txt files?
I see astroquery>=0.4.8.dev0 in the light_curves requirements. Do I remember correctly that if there is any dev version already installed (like on the ISP) it won't be upgraded, but that's still the best option because something like astroquery>=0.4.8.dev9474 won't work? A slightly older version (0.4.8.dev9306) throws an error, so we'd need more than "dev0" to ensure a working version.
There was a problem hiding this comment.
I plan to do a release before the end of the year, so for now update your install manually.
And in the new year astroquery will get a new packaging overhaul and a better system for frequent releases, so no workaround will be needed
|
I'll defer to you both @bsipocz and @troyraen as to which things are important enough to fix and which things aren't really broken. I know Brigitta doesn't want to teach us to ignore warnings, and while my codes may not do it explicitly, my brain certainly does ignore warnings in terms of skimming over them when looking for output. |
and thus we should reduce the noise in tutorial notebooks, and only expose the ones that a user should do something about, as sometimes they contain useful info :) (this is so much easier to say than to do though, I heavily rely on CI turning warning to exceptions at a lot of places, but that works much better in libraries than with notebooks) |
|
The 'divide by zero' warnings are occurring in error calculations when the inverse variance is zero, so the error is infinite. I think we decided in the light curves notebook to leave bad data in. The plotting function in this notebook masks these bad data points, so there's no problem leaving them in from a coding standpoint. So I've used the |
|
@bsipocz I think this is ready to merge. |
| # Inverse variances may be zero, resulting in infinite error. | ||
| # We'll leave these in and ignore the "divide by zero" warning. | ||
| with np.errstate(divide='ignore'): | ||
| error_cgs = np.sqrt(1 / spec["IVAR"][0]) * flux_cgs.unit |
There was a problem hiding this comment.
This maybe a good example for a docs of good practices, e.g. deal with expected Warnings that are due to the nature of the functionality (e.g. either this or the occasional no result warning coming from astroquery when we loop through a list of coordinates where we expect to have no matches!); and any warnings the end user should/can do nothing about.
Deal with warnings 53f71e2
Clean up warnings in the spectra_generator notebook:
NoResultsWarning-- Suppress these since we don't expect all-sky coverage and we explicitly handle the no-results cases.RuntimeWarning: divide by zero-- These occur in error calculations when the inverse variance is zero, resulting in infinite error. Leave this bad data in and ignore the warning.