Repository navigation
[#5408] Don't re-verify invalid FITS cards after their values have been replaced - #11108
Merged
Merged
Conversation
saimn
reviewed
Nov 30, 2020
Contributor
|
Looks good, thanks @embray ! Needs a changelog though. |
Move handling of CONTINUE cards into Card._split() which now parses the value/comment portions of CONTINUE cards into a long "'string' / comment" string which can be parsed as though it were one long value+comment portion of a card. This removes the need to do this from _parse_value and _parse_comment.
I had to rework how Card._verify works: Normally the run_option method, when passed a fix= argument, runs the fix code immediately. However, the fix code can have side-effects that modify the behavior of the later verifications, which makes reasoning about _verify very difficult. So I changed it so that it first collects a list of errors (and potentially errors to fix) and then runs the fixes collectively at the end. It would probably be better if that were the way verify() worked in general, but that is a larger refactoring job to leave for later (possibly can wait until astropy#3668).
Member
Author
|
This was positively reviewed other than missing a changelog entry so I'll go ahead and merge. |
2 tasks done
astrofrog
pushed a commit
that referenced
this pull request
Mar 16, 2021
[#5408] Don't re-verify invalid FITS cards after their values have been replaced
pllim
pushed a commit
to pllim/astropy
that referenced
this pull request
Mar 24, 2021
[astropy#5408] Don't re-verify invalid FITS cards after their values have been replaced
pllim
pushed a commit
to pllim/astropy
that referenced
this pull request
Mar 25, 2021
[astropy#5408] Don't re-verify invalid FITS cards after their values have been replaced
saimn
added a commit
that referenced
this pull request
Mar 27, 2021
Manual backport for #11108 (Don't re-verify invalid FITS cards after their values have been replaced)
embray
added a commit
to embray/astropy
that referenced
this pull request
Apr 6, 2021
It is possible in certain cases for a commentary card (HISTORY or COMMENT) to be stored in a single Card object with a value too long to fit in a single physical FITS card. In these cases the logical card's "image" (its physical representation) is multiple cards concentated together, and neither the _itersubcards() nor _split() methods handled this case properly. For some reason these code paths were previously avoided for these long commentary cards, but after astropy#11108 tried to reuse _split() in more places it did not handle this case properly.
nstarman
pushed a commit
to nstarman/astropy
that referenced
this pull request
Apr 26, 2021
It is possible in certain cases for a commentary card (HISTORY or COMMENT) to be stored in a single Card object with a value too long to fit in a single physical FITS card. In these cases the logical card's "image" (its physical representation) is multiple cards concentated together, and neither the _itersubcards() nor _split() methods handled this case properly. For some reason these code paths were previously avoided for these long commentary cards, but after astropy#11108 tried to reuse _split() in more places it did not handle this case properly.
aaryapatil
pushed a commit
to aaryapatil/astropy
that referenced
this pull request
May 3, 2021
It is possible in certain cases for a commentary card (HISTORY or COMMENT) to be stored in a single Card object with a value too long to fit in a single physical FITS card. In these cases the logical card's "image" (its physical representation) is multiple cards concentated together, and neither the _itersubcards() nor _split() methods handled this case properly. For some reason these code paths were previously avoided for these long commentary cards, but after astropy#11108 tried to reuse _split() in more places it did not handle this case properly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5408.
Fixing this was not so easy because it once again ran up against the huge mess that is FITS card parsing and verification. If I could I would rewrite the whole thing from scratch.