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.

Sign up for GitHub

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

Jump to bottom

Migrate to new asdf converter interface #6057

Merged
merged 19 commits into from
Apr 21, 2022
Merged

Migrate to new asdf converter interface #6057

merged 19 commits into from
Apr 21, 2022

Conversation

Cadair
Copy link
Member

@Cadair Cadair commented Apr 11, 2022 •
edited
Loading

Fixes #6026 and fixes #5859

Todo:

  • Handle missing asdf-astropy

@Cadair Cadair requested a review from a team as a code owner April 11, 2022 14:55
@Cadair Cadair marked this pull request as draft April 11, 2022 14:56
@Cadair Cadair added this to the 4.0.0 milestone Apr 11, 2022
@nabobalis nabobalis added io Affects the io submodule io/ASDF Issues with ASDF labels Apr 11, 2022
@Cadair Cadair force-pushed the asdf_converters branch 3 times, most recently from 723373c to 6ca33a8 Compare April 20, 2022 15:12
@Cadair Cadair marked this pull request as ready for review April 20, 2022 15:35
@Cadair Cadair changed the title Initial implementation of new asdf converter interface Migrate to new asdf converter interface Apr 20, 2022
@Cadair Cadair requested a review from a team as a code owner April 20, 2022 20:02
@Cadair @nabobalis
Apply suggestions from code review
cb299fc 
Co-authored-by: Nabil Freij <nabil.freij@gmail.com>
@Cadair
Copy link
Member Author

Cadair commented Apr 20, 2022

pre-commit.ci autofix

dstansby
dstansby previously requested changes Apr 21, 2022
View reviewed changes
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

I haven't had a chance to check the code, but I've left some doc suggestions, and a request for a changelog entry for changed/added dependencies.

changelog/6057.trivial.rst Outdated Show resolved Hide resolved
docs/code_ref/io.rst Outdated Show resolved Hide resolved
docs/code_ref/io.rst Outdated Show resolved Hide resolved
docs/code_ref/io.rst Outdated Show resolved Hide resolved
docs/code_ref/io.rst Outdated Show resolved Hide resolved
docs/code_ref/io.rst Outdated Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
Cadair and others added 2 commits April 21, 2022 10:15
@dstansby dstansby dismissed their stale review April 21, 2022 09:35

Review comments addressed

Cadair
Cadair commented Apr 21, 2022
View reviewed changes
docs/whatsnew/4.0.rst Outdated Show resolved Hide resolved
@Cadair Cadair added the Needs Review Needs reviews before merge label Apr 21, 2022
@dstansby dstansby mentioned this pull request Apr 21, 2022
Closed
@nabobalis nabobalis removed request for a team April 21, 2022 16:24
@nabobalis nabobalis removed the Needs Review Needs reviews before merge label Apr 21, 2022
@nabobalis nabobalis merged commit 3bff523 into sunpy:main Apr 21, 2022
@Cadair Cadair deleted the asdf_converters branch April 21, 2022 17:07
@pllim pllim mentioned this pull request Apr 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayshih ayshih ayshih left review comments

@nabobalis nabobalis nabobalis approved these changes

@dstansby dstansby dstansby approved these changes

@WilliamJamieson WilliamJamieson WilliamJamieson approved these changes

Assignees
No one assigned
Labels
io/ASDF Issues with ASDF io Affects the io submodule
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.

Fix your hook over at reproject asdf 3.0 will remove support for the ExtensionType plugin format
5 participants
@Cadair @ayshih @nabobalis @dstansby @WilliamJamieson