Skip to content

Fix AsdfFile pickling for instances without file descriptors#2038

Open
sydduckworth wants to merge 10 commits into
asdf-format:mainfrom
sydduckworth:fix-pickle-serialization
Open

Fix AsdfFile pickling for instances without file descriptors#2038
sydduckworth wants to merge 10 commits into
asdf-format:mainfrom
sydduckworth:fix-pickle-serialization

Conversation

@sydduckworth
Copy link
Copy Markdown
Contributor

Description

This PR fixes two bugs that prevented AsdfFile instances from being pickled, which partially addresses #1782.
The result is that now asdf files can be pickled if they don't contain a file descriptor, which basically means files that were created from an in-memory dict.

  • Fixed a bug in which AsdfObject instances were failing to deserialize because of their conflicting dict and UserDict base classes. Added a manual __reduce__ override which resolves the problem.
  • Updated ValidatorManager to not return local callables defined inside a method, which can't be pickled.
    • ValidatorManager now returns a new BoundValidators callable class
    • Significantly simplified ValidatorManager implementation.
  • Added test case to _tests/test_asdf.py to verify the limited pickling that is now supported.

@sydduckworth sydduckworth requested a review from a team as a code owner May 4, 2026 19:21
@sydduckworth sydduckworth requested a review from braingram May 4, 2026 19:24
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How do the changes in this file relate to this PR? If unrelated would you split them into a different PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changing get_jsonschema_validators was necessary to fix the pickling issue.
Because of that change, validate became both unused and untested, so I removed it.
Removing validate allowed me to clean up the constructor for ValidatorManager.

I renamed get_jsonschema_validators to just validators because it is now the only thing that ValidatorManager actually returns, and I found the jsonschema part confusing since it isn't really actually related to jsonschema at all (or any more than the input validators are).

Comment thread asdf/extension/_validator.py
Comment thread asdf/schema.py
Comment thread asdf/extension/_manager.py Outdated
@sydduckworth
Copy link
Copy Markdown
Contributor Author

@braingram I've reverted the public API for ValidatorManager.

Once #2040 is merged I'm going to add @deprecated to ValidatorManager.get_jsonschema_validators.

The issue I'm now running into is that ValidatorManager.validate isn't actually being used or tested anywhere, which is causing the code coverage to drop.
So I could add unit tests for that function, or I could mark it as deprecated and add a coverage exception for deprecated functions?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants