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

load_sample: update amrvac sample data files registry #3280

Merged

Conversation

neutrinoceros
Copy link
Member

@neutrinoceros neutrinoceros commented May 19, 2021

PR Summary

in combination with yt-project/website#97, this should fix #3178

A point that is not clear to me is how to better approach the proposed migration on the testing side. I imaging go as follow:

  • install (download) the renamed datafiles on Jenkins,
  • update yt/frontends/amrvac/tests files
  • drop (delete) the old files on Jenkins

@Xarthisius, any thoughts ?

Since the issue was caused by non standardised packaging, I had to regenerate the hashes for each file.
While I'm at it, I also register the solar_prom2d dataset, which is already on the website but lacking a registration in the pooch registry.

note: I realise that this update breaks alphabetical ordering of the json file entries, but the patch was hard enough to keep track on, so I figure it's preferable to stick to a minimal diff to ease reviews.

@neutrinoceros neutrinoceros added bug code frontends Things related to specific frontends labels May 19, 2021
@neutrinoceros neutrinoceros force-pushed the update_amrvac_testfiles_registry branch from 02c695e to 969aa69 Compare May 19, 2021 20:20
@neutrinoceros
Copy link
Member Author

@matthewturk @jzuhone @Xarthisius I'm pinging you three for reviews since you've all been poking at this in the past, but you don't need to all have a look at this.

@Xarthisius
Copy link
Member

Apart from solar_prom2d that was using the same directory, previous version and current version co-exist for now, so there's no pressure to fixing tests.

@Xarthisius Xarthisius merged commit ab25a3a into yt-project:main Jun 1, 2021
@neutrinoceros
Copy link
Member Author

excellent, thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug code frontends Things related to specific frontends
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: some data samples can't be loaded with yt.load_sample
2 participants