Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only check for HDUs if they are not None (fits reader) #435

Merged
merged 1 commit into from
Dec 13, 2016
Merged

Only check for HDUs if they are not None (fits reader) #435

merged 1 commit into from
Dec 13, 2016

Conversation

MSeifert04
Copy link
Contributor

I've added no regression test (because hdu_flags=None is the default and therefore already checked in several tests) and I added no changelog entry because it shouldn't change any behaviour.

The reason this fails is because None in HDUList is not allowed anymore in astropy.io.fits but hdu_mask, hdu_uncertainty and hdu_flags are allowed to be None.

I reported the issue upstream: astropy/astropy#5583 but I think we should change it even if it's not fixed upstream.

@MSeifert04 MSeifert04 added this to the 1.2 milestone Dec 10, 2016
@MSeifert04 MSeifert04 changed the title Only check for string-like HDUs if they are not None (fits reader) Only check for HDUs if they are not None (fits reader) Dec 10, 2016
@juliantaylor
Copy link

this is a bug in astropy, None in container should imo not raise

@embray
Copy link
Member

embray commented Dec 12, 2016

I agree--it's a bug

@MSeifert04
Copy link
Contributor Author

ok, should be fixed eventually by astropy/astropy#5589.

Thanks for the affirmation @juliantaylor @embray

@MSeifert04 MSeifert04 closed this Dec 12, 2016
@MSeifert04 MSeifert04 deleted the fix-astropy-dev-errors branch December 12, 2016 16:35
@mwcraig
Copy link
Member

mwcraig commented Dec 12, 2016

Hmmm, I was inclined to merge this anyway since there is no need to do the search if the key is None.

@MSeifert04 MSeifert04 restored the fix-astropy-dev-errors branch December 12, 2016 17:08
@MSeifert04
Copy link
Contributor Author

@mwcraig I restored the branch, if you want to reopen and merge it should be possible.

@mwcraig mwcraig reopened this Dec 12, 2016
@mwcraig
Copy link
Member

mwcraig commented Dec 13, 2016

Thanks, merging.

@mwcraig mwcraig merged commit 7b665bc into astropy:master Dec 13, 2016
@MSeifert04 MSeifert04 deleted the fix-astropy-dev-errors branch December 13, 2016 22:40
joshwalawender pushed a commit to joshwalawender/ccdproc that referenced this pull request Feb 2, 2017
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.

4 participants