Fix issues with mixins serialization and FITS#10485
Fix issues with mixins serialization and FITS#10485taldcroft merged 5 commits intoastropy:masterfrom
Conversation
|
|
||
| @pytest.mark.skipif('HAS_YAML') | ||
| def test_warn_for_dropped_info_attributes(tmpdir): | ||
| def test_warn_for_dropped_info_attributes(tmpdir, monkeypatch): |
There was a problem hiding this comment.
Nice, I didn't know about monkeypatch.
|
|
||
| # Copy the meta dict if it was not copied by represent_mixins_as_columns | ||
| if encode_tbl is tbl: | ||
| meta_copy = deepcopy(tbl.meta) |
There was a problem hiding this comment.
At first glance I thought it might be possible to get away without doing a deep copy here. (I have spent a lot of time getting rid of unnecessary deep copies of meta, related to a project where I put a lot of stuff into meta.) But looking more carefully I saw that if the input table had a pre-existing comments list in meta, then that would get updated in place and impact the original meta.
So it might be worth adding a comments key like ['a', 'b'] in the test below. This will then fail if a future maintainer comes along and makes the mistake I almost made. Or at least put in a comment here about why the deepcopy is really necessary.
There was a problem hiding this comment.
Good points, thanks, I did the changes.
Fix issues with mixins serialization and FITS
Fix issues with mixins serialization and FITS
Fix #7783 and fix #7364.