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

API changes for REBOUND version 4 #115

Merged
merged 16 commits into from
Nov 20, 2023
Merged

API changes for REBOUND version 4 #115

merged 16 commits into from
Nov 20, 2023

Conversation

hannorein
Copy link
Collaborator

Staging changes for REBOUND Version 4.

@hannorein
Copy link
Collaborator Author

hannorein commented Nov 10, 2023

I do not know how to temporarily change CI so that it pulls rebound from my version4 branch. Might not be worth doing if we can run the unit tests locally for this PR.

@dtamayo
Copy link
Owner

dtamayo commented Nov 10, 2023

When you push changes to version4 in the rebound repo, the REBOUNDx CI is checking out that version installing locally and then installing REBOUNDx. But yeah, if it doesn't work locally, then it's the same issue on the CI end. When I try to reproduce what you get, I get the same error as the CI spits out, saying implicit declaration of reb_orbit_from_particle. Is that the same issue of not finding the rebound.h file?

@hannorein
Copy link
Collaborator Author

The python units tests are passing now on my local machine. I don't think we can get the CI working before merging at least one of the rebound or reboundx development branches into the main branch. Neither this CI runs triggered by this pull request nor those by pushing to rebound will use the correct combination of "version4" branches. It's either rebound/main and reboundx/version4 or vice versa.

It would be great if you can also confirm that the unit tests are working on your end. I'll hold off with merging version 4 of rebound into the main branch until I get a green light from you. No rush!

@hannorein
Copy link
Collaborator Author

There were as predicted a few hiccups (some examples needed more changes, an issue with Vec3d). But now all tests are passing. Up to you if you want to merge it immediately (I would suggest as a squash commit), or wait a little longer to let things settle...

@dtamayo
Copy link
Owner

dtamayo commented Nov 14, 2023

Thank you!! I'm leaving tomorrow to visit Cornell, so I think I will wait until I get back on Friday when I'll be better able to deal with things that come up

@dtamayo dtamayo marked this pull request as ready for review November 20, 2023 16:35
@dtamayo dtamayo merged commit da72889 into main Nov 20, 2023
12 checks passed
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