Skip to content

Fix write and read FITS tables without ColDefs structure - #12863

Merged
saimn merged 5 commits into
astropy:mainfrom
cmarmo:table-type-5280
Mar 24, 2022
Merged

saimn merged 5 commits into
astropy:mainfrom
cmarmo:table-type-5280

Conversation

@cmarmo

@cmarmo cmarmo commented Feb 16, 2022 •

Copy link
Copy Markdown
Member

Description

This pull request is to address the issues described in #5280 and #5287.
It improve the conversion of table columns from record format spec to FITS format spec in the case of multidimensional objects.

Fixes #5280, fixes #5287.

Thank you for considering it.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the Extra CI label.
  • Is a change log needed? If yes, did the change log check pass? If no, add the no-changelog-entry-needed label. If this is a manual backport, use the skip-changelog-checks label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • Is a milestone set? Milestone must be set but astropy-bot check might be missing; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate backport-X.Y.x label(s) before merge.

@nstarman
nstarman requested a review from saimn February 18, 2022 14:42
@pllim pllim added this to the v5.1 milestone Feb 21, 2022
@cmarmo
cmarmo force-pushed the table-type-5280 branch from 3fc4235 to 9326435 Compare March 4, 2022 01:44
@saimn saimn modified the milestones: v5.1, v5.0.3 Mar 15, 2022
@saimn

saimn commented Mar 15, 2022

Copy link
Copy Markdown
Contributor

Thanks @cmarmo! I'm not sure to really understand the change but it seems to work so fine by me :). Could you add a changelog entry ?

@cmarmo

cmarmo commented Mar 21, 2022 •

Copy link
Copy Markdown
Member Author

I have added a changelog and modified some comments: I hope this will help in clarifying what I believe was the issue.
I have rebased to be sure that all checks pass in the last version... hope this is not a problem.

@saimn saimn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks a lot @cmarmo !

@saimn
saimn merged commit 58887aa into astropy:main Mar 24, 2022
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Mar 24, 2022
@cmarmo
cmarmo deleted the table-type-5280 branch March 24, 2022 21:29
saimn added a commit that referenced this pull request Mar 24, 2022
…863-on-v5.0.x

Backport PR #12863 on branch v5.0.x (Fix write and read FITS tables without `ColDefs` structure)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tables with 2D columns of strings fail round trip to FITS format under py3 io.fits errors with tables containing strings

3 participants