Skip to content

Simplify open#1955

Merged
braingram merged 11 commits into
asdf-format:mainfrom
braingram:simplify_open
Aug 15, 2025
Merged

Simplify open#1955
braingram merged 11 commits into
asdf-format:mainfrom
braingram:simplify_open

Conversation

@braingram

@braingram braingram commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

Description

Simply what happens when asdf.open is called. On main this involves:

This PR simplifies the process to:

  • asdf.open an alias of asdf._asdf.open_asdf
  • calls asdf._io.open_asdf (which reads the comments, tree and blocks)
  • returns a created instance (which still involves a bunch of private attribute assignment but that can be improved in a follow-up PR)

As part of the simplification the following private API changes are made:

  • add asdf._io this may expand in future PR to encapsulate more of the "low-level" IO and is currently the new home of some code that was previously part of AsdfFile including _parse_header_line _read_comment_section etc
  • removes AsdfFile._fname and instead uses AsdfFile._fd._uri (the value that was assigned in asdf.open)
  • removes AsdfFile._pre_write and AsdfFile._post_write
  • removes _get_yaml_content argument to asdf.open (unused)

Requires: astropy/asdf-astropy#290 EDIT: merged

The pytest-asdf plugin had to be fixed as part of this work. This originally revealed some issues with the ndarray examples (where the external block example was only partially tested and the mask example fails to round-trip). See asdf-format/asdf-standard#475 for more details. The approach taken in this PR is to update the plugin to:

  • convert the example to a tagged tree
  • validate the tagged tree using the schema

This avoids any need to roundtrip data or provide blocks (or external blocks) which ultimately improves decoupling between the example testing and the quirks of the python asdf library.

Tasks

  • run pre-commit on your machine
  • run pytest on your machine
  • Does this PR add new features and / or change user-facing code / API? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
    • update relevant docstrings and / or docs/ page
    • for any new features, add unit tests
news fragment change types...
  • changes/<PR#>.feature.rst: new feature
  • changes/<PR#>.bugfix.rst: bug fix
  • changes/<PR#>.doc.rst: documentation change
  • changes/<PR#>.removal.rst: deprecation or removal of public API
  • changes/<PR#>.general.rst: infrastructure or miscellaneous change

@braingram braingram marked this pull request as ready for review August 13, 2025 14:09
@braingram braingram requested a review from a team as a code owner August 13, 2025 14:09

@zacharyburnett zacharyburnett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this LGTM

@braingram braingram merged commit c6b24a5 into asdf-format:main Aug 15, 2025
71 of 73 checks passed
@braingram braingram deleted the simplify_open branch August 15, 2025 14:17
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