Repository navigation
wcslib 6.2 update - #8592
wcslib 6.2 update#8592
Conversation
|
@nden - can you rebase to make sure we are testing against the latest master? (I think this will fix the 32-bit failure) |
Codecov Report
@@ Coverage Diff @@
## master #8592 +/- ##
==========================================
- Coverage 86.82% 86.63% -0.19%
==========================================
Files 386 386
Lines 58197 58411 +214
Branches 1060 1096 +36
==========================================
+ Hits 50531 50606 +75
- Misses 7051 7178 +127
- Partials 615 627 +12
Continue to review full report at Codecov.
|
|
I'm not sure what'r wrong with circleci, it seems to run on your account/fork rather than on astropy. It also notoriously known as not working well with submodules, and the job is failing something helpers related. I would suggest to merge the PR if everything else passes, and if there are real 32bit issues on master, patch it up in a follow-up PR. |
|
@bsipocz Would it help if I try a fresh PR based on master later today and see if it resolves the issue? |
|
@nden - honestly I don't know. I see this happened on circleCI before, but not yet identified the cause. So I suggest if you could fix the docs here, and we merge at a point when you foresee to have time to do a follow-up if needed. |
|
Rebasing on the latest master would be good if this isn't already the case. |
|
@astrofrog This failure is after a rebase. |
|
Ok yeah I'm not sure then, as @bsipocz said CircleCI does weird things. Let's just ignore it, and focus on the docs build. |
|
This will need to be reviewed carefully as I am not very familiar with the package at that level. |
astrofrog
left a comment
astrofrog
left a comment
There was a problem hiding this comment.
This looks good to me - of course I didn't check all the WCSLIB changes themselves, but the changes in astropy.wcs itself look good. I tested this with a time cube I have and it worked fine. Just a small comment about the changelog below but approving already.
Once this is in, I'll work on making the APE 14 WCS class return proper Time objects for time coordinates, though that will likely not be done in time for v3.2 (which is ok).
| astropy.wcs | ||
| ^^^^^^^^^^^ | ||
|
|
||
| - updated wcslib to v 6.2 [#8592] |
There was a problem hiding this comment.
I think it would be worth mentioning the improvement that the WCS class can now be used to parse headers that contain time axes?
Also super minor, I'd fix the capitalization as Updated WCSLIB to v6.2
|
Running the tests one more time to make sure the conflivts were resolved correctly. |
|
It'll take a while to get this run on 32bit, but nevertheless we shall keep an eye on that build: |
I intend to write more tests but submitting this for initial review.
Resolves #7929