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

deprecate asdf.open(copy_arrays=True) in favor of asdf.open(memmap=False) #157

Merged
merged 6 commits into from
Nov 1, 2024

Conversation

zacharyburnett
Copy link
Contributor

@zacharyburnett zacharyburnett commented Oct 31, 2024

deprecation for asdf>=4.0

@lgarrison
Copy link
Member

Thank you! Does this imply a higher minimum asdf version? Right now we're at >=2.8.

@zacharyburnett
Copy link
Contributor Author

zacharyburnett commented Oct 31, 2024

Thank you! Does this imply a higher minimum asdf version? Right now we're at >=2.8.

ah! yes, memmap was introduced asdf>=3.1.0. I can rework this to dynamically check the asdf version, if that's desirable (if you have users using asdf<3.1.0) or we can up the minimum version

@braingram
Copy link

braingram commented Oct 31, 2024

Also FYI memmap will switch it's default to False in asdf 4.0.0 so it might not be necessary to provide it for asdf >= 4.0.0.dev since all the uses I see here are memmap=False.

xref: asdf-format/asdf#1801

@zacharyburnett
Copy link
Contributor Author

Also FYI memmap will switch it's default to False in asdf 4.0.0 so it might not be necessary to provide it for asdf >= 4.0.0.dev since all the uses I see here are memmap=False.

there is one usage of memmap=True that I missed: #157 (comment)

@lgarrison
Copy link
Member

I think setting the minimum to asdf>=3.1.0 would be fine.

@lgarrison lgarrison merged commit 65ab028 into abacusorg:main Nov 1, 2024
8 checks passed
@lgarrison
Copy link
Member

Thanks @zacharyburnett and @braingram!

@zacharyburnett zacharyburnett deleted the deprecate/asdf_copy_arrays branch November 1, 2024 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants