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

Remove numpy hack in io.fits when Numpy minversion is 1.12 #7545

Closed
pllim opened this issue Jun 6, 2018 · 5 comments · Fixed by #7058
Closed

Remove numpy hack in io.fits when Numpy minversion is 1.12 #7545

pllim opened this issue Jun 6, 2018 · 5 comments · Fixed by #7058

Comments

@pllim
Copy link
Member

pllim commented Jun 6, 2018

I stumbled upon io/fits/_numpy_hacks.py, which has a comment that says it can be retired when numpy/numpy#6430 is merged. That Numpy fix was incorporated into Numpy 1.12, which means we can remove this hack when our minversion for Numpy is 1.12.

(I am tempted to set this to novice but I probably shouldn't.)

@bsipocz
Copy link
Member

bsipocz commented Jun 6, 2018

Given #7058, I'm milestoning this 3.1 to not to forget about it.

@bsipocz bsipocz added this to the v3.1 milestone Jun 6, 2018
@astrofrog
Copy link
Member

The best approach would probably be to modify the source code to explicitly have NUMPY_LT_112 so that we can then search/replace whenever we update the minimum Numpy version.

@astrofrog
Copy link
Member

This is not 'upstream fix required' since the fix has already been made, right?

@pllim
Copy link
Member Author

pllim commented Jun 7, 2018

Oh, I guess you don't need that label... I just wanted something to indicate that this depends on upstream changes but I guess it is unnecessary given the milestone.

@pllim
Copy link
Member Author

pllim commented Jun 14, 2018

modify the source code to explicitly have NUMPY_LT_112

Seems unnecessary now, as @mhvk has removed it as part of #7058 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants