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

Suppress errors when creating WCS in CCDData reader #6500

Merged
merged 2 commits into from Sep 12, 2017

Conversation

@MSeifert04
Copy link
Contributor

MSeifert04 commented Sep 1, 2017

Fixes #6499

However I'm not sure what I need to do with the Header so it actually fails so I haven't included a regression test.

@astropy-bot

This comment has been minimized.

Copy link

astropy-bot bot commented Sep 1, 2017

Hi there @MSeifert04 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labelled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

Everything looks good from my point of view! 👍

@MSeifert04 MSeifert04 force-pushed the MSeifert04:ccddata_reader_wcs_can_fail branch from d679179 to a980a19 Sep 1, 2017
@pllim pllim added nddata wcs labels Sep 1, 2017
@pllim pllim requested a review from mwcraig Sep 1, 2017
@pllim

This comment has been minimized.

Copy link
Member

pllim commented Sep 1, 2017

Is this considered a bug fix or API change?

@MSeifert04

This comment has been minimized.

Copy link
Contributor Author

MSeifert04 commented Sep 1, 2017

Basically a bug fix because the current code is based on the assumption that WCS cannot fail. This is stated as code comment which turned out to be wrong.

@MSeifert04

This comment has been minimized.

Copy link
Contributor Author

MSeifert04 commented Sep 8, 2017

@bsipocz It's a fairly simple fix and is only missing reviewers. Would it be possible to still include it in 2.0.2?

@bsipocz

This comment has been minimized.

Copy link
Member

bsipocz commented Sep 8, 2017

I'm in the final stages of testing the release, so I'm afraid this needs to wait for 2.0.3.

@MSeifert04

This comment has been minimized.

Copy link
Contributor Author

MSeifert04 commented Sep 8, 2017

@bsipocz okay, no worries. Would've been nice to get this in because it's an almost trivial bugfix.

@MSeifert04 MSeifert04 force-pushed the MSeifert04:ccddata_reader_wcs_can_fail branch from 3fb82c8 to c43f048 Sep 9, 2017
@MSeifert04

This comment has been minimized.

Copy link
Contributor Author

MSeifert04 commented Sep 12, 2017

@mwcraig @crawfordsm Could you have a look at the fix?

@crawfordsm crawfordsm self-requested a review Sep 12, 2017
@crawfordsm

This comment has been minimized.

Copy link
Member

crawfordsm commented Sep 12, 2017

Ah this is great to see as I've run into this problem very recently. It's a pretty straight forward fix that you have put in and I'd agree this is a bugfix as it shouldn't be a problem due to wcs suppose to always be working. I've check it out and can confirm that it works, so happy to see it merged.

Thanks @MSeifert04 !

@bsipocz bsipocz merged commit f87a3e5 into astropy:master Sep 12, 2017
5 checks passed
5 checks passed
astropy-bot All checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.001%) to 83.678%
Details
@bsipocz

This comment has been minimized.

Copy link
Member

bsipocz commented Sep 12, 2017

Thanks @MSeifert04!

@MSeifert04 MSeifert04 deleted the MSeifert04:ccddata_reader_wcs_can_fail branch Sep 12, 2017
bsipocz added a commit that referenced this pull request Sep 26, 2017
Suppress errors when creating WCS in CCDData reader
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.