Deprecate AsdfFile.version_map
#1745
Conversation
asdf/_asdf.py
Outdated
| @@ -807,7 +809,7 @@ def _open_asdf( | |||
| msg = "Does not appear to be a ASDF file." | |||
| raise ValueError(msg) from e | |||
| self._file_format_version = cls._parse_header_line(header_line) | |||
| self.version = self._file_format_version | |||
| self.version = self.file_format_version | |||
There was a problem hiding this comment.
This seems incorrect. self.version is the asdf standard version yet here it is being set to the file_format_version (which is always 1.0.0). self.version is overwritten for probably every file a few lines down (from the undocumented ASDF_STANDARD comment). As the ASDF standard does not define the comment or what to do in this case I'm inclined to leave in this behavior for this PR (as it's only tangentially related to deprecating version_map).
It is likely an improvement to change this to default to the default_version if the file doesn't define a version. However this would be a major change (switching from asdf standard 1.0.0 to 1.5.0) if there is any code that relies on this.
There was a problem hiding this comment.
My guess is that early (possibly pre-release) versions of ASDF did not include an #ASDF_STANDARD comment and this line is here to allow those files to be read.
There was a problem hiding this comment.
Good guess!
I changed this to just default to 1.0.0 if no #ASDF_STANDARD comment is found. That way it should hopefully not break for 2.0.0.
There were 2 tests that failed following this change. One was filling in the standard version for #ASDF (so should have been producing an error but was not), the other was expecting an unsupported standard version warning when the #ASDF version was incorrect. Both of these were updated. I wouldn't be surprised if there are more places where these two things are conflated :-/
| @@ -408,62 +402,3 @@ def assert_no_warnings(warning_class=None): | |||
| assert not any(isinstance(w.message, warning_class) for w in recorded_warnings), display_warnings( | |||
| recorded_warnings, | |||
| ) | |||
|
|
|||
|
|
|||
| def _assert_extension_type_correctness(extension, extension_type, resolver): | |||
There was a problem hiding this comment.
This was unused so it was removed instead of updating it to not use get_version_map.
braingram
force-pushed
the
version_map
branch
3 times, most recently
from
16c821e
to
44fe69f
Compare
| @@ -286,57 +275,6 @@ def _assert_roundtrip_tree( | |||
| asdf_check_func(ff) | |||
|
|
|||
|
|
|||
| def yaml_to_asdf(yaml_content, yaml_headers=True, standard_version=None): | |||
There was a problem hiding this comment.
This was replaced with asdf.testing.helpers.yaml_to_asdf instead of updating it to remove the use of version_map.
braingram
force-pushed
the
version_map
branch
from
3410660
to
deca3d9
Compare
braingram
force-pushed
the
version_map
branch
2 times, most recently
from
a6a8b6a
to
183afb7
Compare
Description
This PR:
AsdfFile.version_mapversioning.get_version_map(which this PR also renames to_get_version_mapas it is undocumented, not listed inversioning.__all__and unused by all known downstream packages)Note that at the moment ASDF only supports file format 1.0.0 and yaml 1.1. This PR adds those constants to
asdf.versioning.When this PR is merged a new issue can be opened to track the removal of
AsdfFile.version_mapin asdf 4.0. The PR that removesversion_mapcan also removeversioning._get_version_map. Once that PR is merged, the version map files can be removed from asdf standard.sunpy downstream is failing due to pytest 8
weldx is failing due to new pandas deprecations
Checklist: