-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update selecteph
#32
Update selecteph
#32
Conversation
Are there more things you would like to add here @LuEdRaMo, or is this ready for review/merging? |
This is ready, altough I don't know if we should merge this before/after #35. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, thanks for this contribution @LuEdRaMo. I would only suggest to add a quick unit test probably after the calls to propagate
in the tests, just as a quick check on the functionality. Otherwise, LGTM!
Couple of other points: we should try to update this branch with latest changes in |
Alright, LGTM! Merging! Thanks! |
This PR updates the function
selecteph
to cover the functionality of NEOs'loadpeeph
, so that we can eventually deprecate the latter.