Updated SIA up-casing behavior #545
Conversation
There was a problem hiding this comment.
As commented in #544, I think this is a welcome robustness patch.
You should address the style issues, though, and I think a changelog entry "More robust handling of SIA1 FORMAT" would be in order. I am pretty sure that passing in bytes would have failed before anyway, so I don't think dropping it needs mentioning.
The failing tests look unrelated; I suppose we need to work on the robustness of the doctests...
Whoever merges this: My vote is that this should be squash-merged.
There was a problem hiding this comment.
Minor fixes: please remove the debug line, it should fix the test, and fix the style things. You can run flake8 pyvo --count locally to see what they are until the count drops down to 0.
Also, please rebase. Currently, you have each of the commits twice in this PR. A rebase workflow will help mainaining a clean commit history by not allowing multiple versions of the same branches entangled, and it also keeps a clean resolution for commit conflicts, too. I'm happy to walk you through a rebase if you want.
|
We don't need to worry about the changelog failure above, I can move the entry to the bugfix section when/if I'm cutting a bugfix release (not yet sure we will have that or go ahead with the feature release sometimes soon). |
|
Re: #545 (comment) Added. Hope it helps! |
|
Given the imminent 1.5.2 release, I go ahead and rebase this to fix the conflict, and rerun with a fixed CI. |
Updated SIA format parameter typesetting
…cific handling Style change: renamed variables Added line to changelog
bsipocz
dismissed
msdemlei’s stale review
Review comments have been addressed, going ahead with the merge to have this in the new bugfix release.
Updated SIA up-casing behavior
Updated SIA up-casing behavior
Modified the behavior of the SIA service where it would upcase all format keywords. Modified to upcase keywords "ALL", "METADATA", and "GRAPHIC", otherwise the parameter is passed unchanged. The format "GRAPHIC-xyz" will capitalize "GRAPHIC" and leave the rest unchanged.
Tests have been added to check that the formats are cased as expected. Variable names have been updated to be more descriptive.
Edit: changed description of PR to reflect updates.