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

gala -> galax conversion #79

Merged
merged 3 commits into from
Jan 25, 2024
Merged

Conversation

nstarman
Copy link
Contributor

@nstarman nstarman commented Jan 21, 2024

Support for UnitSystem and most potentials.

This is the simple and verbose solution.
A fair number of the conversions can be abstracted over when galax.AbstractPotential gets a .parameters attribute.
But more generally I think this interoperability should be moved to a separate package and registered in using entry points.
I'd also like to add a to/from_format descriptor object to galax.AbstractPotential, like is on astropy.Cosmology for a very general convenience layer for object conversion.

@nstarman nstarman added this to the v0.1 milestone Jan 21, 2024
@nstarman nstarman marked this pull request as ready for review January 21, 2024 20:31
@nstarman nstarman requested a review from adrn January 21, 2024 20:36
@adrn
Copy link
Collaborator

adrn commented Jan 22, 2024

I need to think about this more, but we might not want to support a public-facing Galax -> Gala conversion. We definitely want to support Gala -> Galax, because people may want to convert their old code to Galax. But given that Galax will be much more flexible in terms of the kinds of parameters you can specify, it might not be worth supporting the inverse!

@nstarman
Copy link
Contributor Author

SGTM. I'll move galax_to_gala into the tests (for round-tripping) and keep gala_to_galax here.

@nstarman nstarman force-pushed the test-gala branch 4 times, most recently from ac8ab73 to 9e072d3 Compare January 24, 2024 00:06
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
@nstarman nstarman force-pushed the test-gala branch 4 times, most recently from f0114c4 to 150ef36 Compare January 25, 2024 16:24
@nstarman
Copy link
Contributor Author

Need to refactor the tests to test this properly.

Signed-off-by: nstarman <nstarman@users.noreply.github.com>

optional dependency

Signed-off-by: nstarman <nstarman@users.noreply.github.com>
Copy link

codecov bot commented Jan 25, 2024

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@nstarman nstarman changed the title galax <-> gala conversion gala -> galax conversion Jan 25, 2024
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
@nstarman nstarman merged commit a925ea4 into GalacticDynamics:main Jan 25, 2024
13 checks passed
@nstarman nstarman deleted the test-gala branch January 25, 2024 19:08
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.

None yet

2 participants