Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce asdf.util.load_yaml #1700

Merged
merged 7 commits into from
Jan 15, 2024
Merged

Conversation

braingram
Copy link
Contributor

@braingram braingram commented Dec 8, 2023

Description

This PR introduces asdf.util.load_yaml to load the yaml contents of an ASDF file.

import asdf

# load a tree containing, lists, dicts and strings (no tags)
tree = asdf.util.load_yaml("test.asdf")

# load a tree containing asdf.tagged.Tagged instances
tagged_tree = asdf.util.load_yaml("test.asdf", tagged=True)

This is an alternative to #1677 and can be used to replace the (many) uses of _force_raw_types in downstream code and _get_yaml_content (see note below about sphinx-asdf).

It should be possible to remove _force_raw_types once all the downstream packages have removed its usage. This PR removes the use of this argument in asdftool edit. asdftool diff will require more work to remove the use of _force_raw_types. At the moment this PR does not deprecate nor remove _force_raw_types.

Although _get_yaml_content is no longer used in asdf with this PR it is used in sphinx-asdf and will require a release and fix (see asdf-format/sphinx-asdf#87) and release of sphinx-asdf before removal.

Checklist:

  • pre-commit checks ran successfully
  • tests ran successfully
  • for a public change, a changelog entry was added
  • for a public change, documentation was updated
  • for any new features, unit tests were added

@braingram braingram changed the title Load yaml Introduce asdf.util.load_yaml Dec 8, 2023
@braingram braingram force-pushed the load_yaml branch 2 times, most recently from 48982d6 to 877a716 Compare December 8, 2023 23:15
@braingram braingram marked this pull request as ready for review December 8, 2023 23:19
@braingram braingram requested a review from a team as a code owner December 8, 2023 23:19
Copy link
Contributor

@eslavich eslavich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, just the one question about BaseLoader.

if tagged:
loader = AsdfLoader
else:
loader = yaml.CBaseLoader if getattr(yaml, "__with_libyaml__", None) else yaml.BaseLoader
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does BaseLoader do when it encounters a tag that it doesn't know how to handle? I thought I remembered that it throws an error in that case but clearly not...

Copy link
Contributor Author

@braingram braingram Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look!

My understanding is the BaseLoader ignores all tags. The pyyaml documentation has a rather brief description:

BaseLoader(stream) does not resolve or support any tags and construct only basic Python objects: lists, dictionaries and Unicode strings.

Throwing an empty ASDF file at it:

#ASDF 1.0.0
#ASDF_STANDARD 1.5.0
%YAML 1.1
%TAG ! tag:stsci.edu:asdf/
--- !core/asdf-1.1.0
asdf_library: !core/software-1.0.0 {author: The ASDF Developers, homepage: 'http://github.com/asdf-format/asdf',
  name: asdf, version: 3.0.2.dev46+gfc646c3c}
history:
  extensions:
  - !core/extension_metadata-1.0.0
    extension_class: asdf.extension._manifest.ManifestExtension
    extension_uri: asdf://asdf-format.org/core/extensions/core-1.5.0
    software: !core/software-1.0.0 {name: asdf, version: 3.0.2.dev46+gfc646c3c}
...

with:

d = yaml.load(open('foo.asdf').read(), Loader=yaml.BaseLoader)

yields:

> d
{'asdf_library': {'author': 'The ASDF Developers',
  'homepage': 'http://github.com/asdf-format/asdf',
  'name': 'asdf',
  'version': '3.0.2.dev46+gfc646c3c'},
 'history': {'extensions': [{'extension_class': 'asdf.extension._manifest.ManifestExtension',
    'extension_uri': 'asdf://asdf-format.org/core/extensions/core-1.5.0',
    'software': {'name': 'asdf', 'version': '3.0.2.dev46+gfc646c3c'}}]}}

inspecting the extension item:

> type(d['history']['extensions'][0])
dict

In the pyyaml source BaseLoader uses BaseConstructor for building objects and BaseConstructor is the parent class for SafeConstructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants