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

Fix order of Athena++ cylindrical coords #4801

Merged

Conversation

forrestglines
Copy link
Contributor

PR Summary

Changes the ordering of coordinates for Athena++ cylindrical datasets to $(r,\theta,z)$ within the Athena++ frontend.

Addresses #4796

PR Checklist

  • New features are documented, with docstrings and narrative docs
  • Adds a test for any bugs fixed. Adds tests for new features.

Copy link

welcome bot commented Feb 1, 2024

Hi! Welcome, and thanks for opening this pull request. We have some guidelines for new pull requests, and soon you'll hear back about the results of our tests and continuous integration checks. Thank you for your contribution!

@forrestglines forrestglines changed the title WIP: Fix order of cylindrical coords, GR broke WIP: Fix order of Athena++ cylindrical coords Feb 1, 2024
@forrestglines forrestglines marked this pull request as draft February 1, 2024 21:52
@forrestglines
Copy link
Contributor Author

Currently this PR changes to coordinate order for Athena++ cylindrical datasets and also fixes it for Cartesian and spherical polar dataset.

I need to update it to work for GR coordinates. Final version will leave everything untouched except for cylindrical

matthewturk
matthewturk previously approved these changes Feb 2, 2024
yt/frontends/athena_pp/data_structures.py Show resolved Hide resolved
Co-authored-by: Clément Robert <cr52@protonmail.com>
@forrestglines forrestglines changed the title WIP: Fix order of Athena++ cylindrical coords Fix order of Athena++ cylindrical coords Feb 5, 2024
@forrestglines forrestglines marked this pull request as ready for review February 5, 2024 20:59
@forrestglines
Copy link
Contributor Author

I think this is ready to merge. assuming the CI is working+ passing and there are no other changes.

@neutrinoceros
Copy link
Member

pre-commit.ci autofix

@neutrinoceros neutrinoceros added bug code frontends Things related to specific frontends coordinates: non-cartesian labels Feb 6, 2024
@neutrinoceros
Copy link
Member

@yt-fido test this please

@matthewturk
Copy link
Member

lgtm!

@matthewturk matthewturk merged commit 75b04d8 into yt-project:main Feb 15, 2024
12 of 13 checks passed
Copy link

welcome bot commented Feb 15, 2024

Hooray! Congratulations on your first merged pull request! We hope we keep seeing you around! 🎆

@neutrinoceros neutrinoceros added this to the 4.3.1 milestone Feb 15, 2024
@neutrinoceros
Copy link
Member

@meeseeksdev backport to yt-4.3.x

@neutrinoceros
Copy link
Member

@matthewturk please remember to milestone bug fix PRs before merging. 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 coordinates: non-cartesian
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants