Repository navigation
Fix behavior of scaled columns in binary table - #6956
Conversation
|
Hi there @saimn 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃. Everything looks good from my point of view! 👍 If there are any issues with this message, please report them here. |
|
needs a rebase because of the conflict. Also: Why is there a significant decrease in coverage? 😕 |
|
for changelog issue, no need to rebase now only when it's ready to be merged (more changes are expected with 2.0.3) |
Hmm I think the code removal explains this (as it was probably well tested) ? roughly 40 lines / 100000 gives 0.04% |
|
Another possibility is that the top of master changed with significant changes between opening this and the actual run of the coverage test. |
|
Yes maybe, we can see when I rebase the branch. |
|
2.0.3 is out, so this can be now rebased. |
452fca3 to
82e7127
Compare
|
@drdavella - This commit was meant to reproduce the issue, but it seems that it is not exactly the same as this test now pass but I can still reproduce the error described in #3921. But anyway I prefer to leave this for another round, I think it's better to have the test with the corresponding fix (or the opposite ;)). Otherwise I rebased to be sure the changelog is there for 2.0.4. |
Once a column with a tscale or tzero keyword is scaled, it was not marked as such. So if the column was modified and then saved, the old (unscaled) values when saved instead. It seems that all what was missing was to put the `_physical_values` flag to True.
…hat always nice? This handling of unsigned ints already happens in FITS_rec.from_columns, by way of Column._convert_to_valid_data_type.
…unsigned-int columns.
82e7127 to
7d44b58
Compare
|
ping @MSeifert04 @drdavella - is this fine for you ? What's the policy for v3.0 inclusion now that it is branched ? It can go to v3.0.1 if needed, but it is "just" a bugfix. |
|
@saimn - bugfixes can still go into 3.0.0, especially everything that goes into 2.0.4 should actually end up being in 3.0.0, too (unless 2.0.x specific, and we don't have that many of those). |
|
@bsipocz - Ok great, so I just need someone to approve 😉 (ping @drdavella as you already had a look) |
|
@drdavella @MSeifert04 - ping ! |
Fix behavior of scaled columns in binary table
Fix behavior of scaled columns in binary table
|
Thanks, good catch (I should be more careful when using "fix" in the issue description!). |
|
Thanks @saimn |
Fix #6887.
Once a column with a tscale or tzero keyword is scaled, it was not marked as such. So if the column was modified and then saved, the old (unscaled) values when saved instead.
It seems that all what was missing was to put the
_physical_valuesflag to True.While looking at old issues I found #4709 which is very similar, so I included these 2 commits. Sadly it is not enough to fix #3921 (I still do not understand everything there 🙄 )
Note: There is no 2.0.4 section in the changelog yet, so I used 2.0.3 for now.