Repository navigation
Fix _naxis dimensionality - #18419
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
|
👋 Thank you for your draft pull request! Do you know that you can use |
astrofrog
left a comment
There was a problem hiding this comment.
This seems sensible from my point of view
|
@ayshih I don't think we need to hide Since @astrofrog did a lot of fixes in his PR + unit tests, would you like to implement my suggested fix in your PR? or will you rebase your PR after this (simplified PR without your ellipses fixes and tests) is merged? |
I guess I'm ambivalent now. It was problematic when the line was always |
I agree. I do not often work with 3D WCS but I noticed this now and didn't like it. I think it is OK to remove it since the code already prints and also the number of axes can be inferred from the shape of the PC or CD matrix or CRPIX. That is, does not really provide any new information. Eh, after I wrote this, I think it does make it easier to see |
|
@astrofrog Feel free to port this fix to your PR and then close this or accept/merge this and rebase yours. |
…419-on-v7.1.x Backport PR #18419 on branch v7.1.x (Fix _naxis dimensionality)
Description
This PR fixes, I believe, the root-cause of issues/PRs reported in #18405 and #18417. It borrows some code from #18417 (mostly testing and ellipses stuff just so that we can test that proposed changes fix the issues reported in #18417) however, the purpose of this draft PR is to illustrate what I meant in the last paragraph in #18417 (comment) and if @astrofrog finds this solution acceptable, I think this fix should go into #18417 (alternatively this could deal with
_naxisdimensionality only and #18417 with ellipses after rebasing).Would these changes also fix #18405, @ayshih ?
Close #18417