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

Work to enable interfaces with different-than-standard index lengths #1080

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rieder
Copy link
Member

@rieder rieder commented Oct 14, 2024

Work for issue #1077

  • Change gd.py to a module for flexibility
  • Create a "GravitationalDynamics64Interface" that inherits from "GravitationalDynamicsInterface" but changes the indexes to 64 bit?

@rieder
Copy link
Member Author

rieder commented Oct 21, 2024

@LourensVeen I think this solution works. I wish it could be done with less code repetition, but I don't immediately see how...

@LourensVeen
Copy link
Collaborator

As a point of terminology: you converted the gd.py module to a gd/ package. A submodule is a git thing that is nice in theory but a pain in practice, and also package here should not be confused with the wheels or tarballs (or, a long time ago, eggs, except that they're still around somewhat too) that you upload to PyPI, which are also packages but of a different kind. Python...

(This comment brought to you by having spent the day wading through the amuse.rfi code which is horribly inconsistently named and frustrating to work with, sorry for taking it out on your PR 😬)

I'm not sure the subdirectory is all that necessary actually, to clean up amuse.community.interface I would start with moving it to amuse.interface and moving the example.py to the documentation. That would leave six files, which would become eight, which is no big deal at all. If there's a split, I'd do it by field, but then we'd probably get amuse.interface and omuse.interface and such.

Copy-pasting and modifying is ugly, but I didn't find a different solution either, and if there is one it may well be worse. I'd like to replace the whole mess that is amuse.rfi with a new implementation and if I end up doing that, then I may be able to design in a better solution for this. Meanwhile it may have to do, unfortunately.

@rieder
Copy link
Member Author

rieder commented Oct 31, 2024

ok to merge, or are changes needed?

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.

2 participants