ejeschke / ginga

The Ginga astronomical FITS file viewer
BSD 3-Clause "New" or "Revised" License
120 stars 77 forks source link

Add option to suppress FITS verify warnings from astropy.io.fits #1010

Closed ejeschke closed 2 years ago

ejeschke commented 2 years ago

This PR adds an option to suppress FITS verification warnings when using astropy.io.fits as the FITS file opener. It is opt-in, and can be invoked by adding --suppress-fits-warnings to the command line, or by adding suppress_fits_warnings = True to the general.cfg configuration file in the user's $HOME/.ginga folder.

ejeschke commented 2 years ago

ping @profxj

profxj commented 2 years ago

Thanks for this!

pllim commented 2 years ago

Does not seem to work for me when I do this using the file provided in https://github.com/ejeschke/ginga/issues/997#issuecomment-1055981688 . Does it work for you? Is it just me?

ginga --loglevel=30 --stderr --suppress-fits-warnings <filename>
ejeschke commented 2 years ago

Does it work for you? Is it just me?

@pllim, it works for me. Double check the install and env?

ejeschke commented 2 years ago
IPython 7.12.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from astropy import __version__                                         

In [2]: __version__                                                             
Out[2]: '4.0'

Are you using astropy 5? Did something change there?

pllim commented 2 years ago

I am using astropy 5.x series. astropy v5.0 is the new LTS, which means astropy v4.x and older are no longer supported. Does it still work for you if you upgrade astropy?

pllim commented 2 years ago

I have Python 3.9.11 on Windows 10 (via Git Bash), astropy v5.1rc1 (but v5.1 is available now, so you don't have to use the RC), and Ginga from this branch.

ejeschke commented 2 years ago

It works for me with astropy 5.0. Will try 5.1.

ejeschke commented 2 years ago

Hold that, having inconsistent results with 5.0. Stand by...

ejeschke commented 2 years ago

No, I stand by that. Working with 5.0. Got confused for a moment when I ran the non-PR version.

pllim commented 2 years ago

Hmm! I will try again as well and report back...

ejeschke commented 2 years ago

Works for me with astropy=5.1 as well.

ejeschke commented 2 years ago

Both command line option and putting a setting in $HOME/.ginga/general.cfg

pllim commented 2 years ago

I upgraded from astropy v5.1rc1 to v5.1 but it still does not work for me. I wonder if this is Windows specific or there is something wrong in my local setup. Did you test this on Windows?

I still see these stuff on the terminal:

...
2022-06-15 17:17:09,122 | W | warnings.py:109 (_showwarnmsg) |
C:\...\astropy\io\fits\card.py:264: VerifyWarning:Keyword name 'FRAME9 _ATE'
is greater than 8 characters or contains characters not allowed by the FITS standard;
a HIERARCH card will be created.
  warnings.warn(

2022-06-15 17:17:09,122 | W | warnings.py:109 (_showwarnmsg) |
C:\...\astropy\io\fits\card.py:264: VerifyWarning:Keyword name 'FRAME9 GAIA'
is greater than 8 characters or contains characters not allowed by the FITS standard;
a HIERARCH card will be created.
  warnings.warn(

2022-06-15 17:17:09,123 | W | warnings.py:109 (_showwarnmsg) |
C:\...\astropy\io\fits\card.py:264: VerifyWarning:Keyword name 'PSF_FWHM_ERR'
is greater than 8 characters or contains charactersnot allowed by the FITS standard;
a HIERARCH card will be created.
  warnings.warn(
pllim commented 2 years ago

I just cannot get this to work on my end. Not sure why. If @profxj is happy with this, then I guess it is good enough, since I don't deal with the affected data. I would recuse myself from the review process if this works for everyone else.

ejeschke commented 2 years ago

Hmm, I don't see why Windows should be different as far as suppressing Python warnings. Did/can you try this on your RH workstation at STScI?

ejeschke commented 2 years ago

I see the problem: it only takes effect if the FITSpkg option is set to something other than "choose". I should remember to set my GINGA_HOME to an empty folder prior to testing.

I'll push an update shortly.

ejeschke commented 2 years ago

Ok, @pllim, sorry for the churn and thanks for being persistent--it paid off! Could you try the PR now? I think you will find it works as advertised.

I'm hopeful we can find a better solution in the future that would suppress things on a file by file basis, but we may need some work on the astropy side for the fits module. It's definitely a concern for loading VLT files if it consumes so much time to issue warnings as discussed in #997. I guess one idea would be not to call verify().

pllim commented 2 years ago

Update: --suppress-fits-warnings seems to work but not when I just set it in ~/.ginga/general.cfg without using --suppress-fits-warnings.

pllim commented 2 years ago

Here is my general.cfg -- general.cfg.txt

ejeschke commented 2 years ago

Here is my general.cfg -- general.cfg.txt

Thanks for being so thorough, @pllim. I found that you have an error in your general.cfg: line 97 should begin with a comment character but does not. I think this somehow is messing up the reading of the config file although no error is raised. If I comment that line by prefixing a '#' then the suppress behavior works. Can you let me know if this changes things for you?

And I think we need to make a new issue for this. :disappointed:

pllim commented 2 years ago

(I'll let you do the merge.)

ejeschke commented 2 years ago

Thanks for the review!