-
Notifications
You must be signed in to change notification settings - Fork 25
frebin and fshift functions #87
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
Conversation
|
I'm not an IDL guru, but I don't see fshift in the IDL astrolib function list here or on the github here. Can you provide a link to the documentation for the original IDL function? I generally support adding new functions to this library, so thanks for the PR! It is currently JWST proposal season so I don't have time to do a full code review at the moment, but I noticed a few small things you could improve:
|
|
Thanks for taking the time to look at this during busy proposal season! The original IDL fshift function is archived here: https://asd.gsfc.nasa.gov/archive/idlastro/ftp/contrib/malumuth/fshift.pro. I also updated the compat requirements and dimensionality checking as suggested. |
|
Would it be possible to add tests for the functions? I think the Codecov bot is identifying some if-else branches which are not covered under test cases. |
|
Just added a few more tests - it seems like everything is being covered now. |
|
Thank you so much! |
I've added implementations of the frebin and fshift functions. I've tested a few cases and it seems to all agree with the IDL version. frebin requires the Interpolations.jl package as a new dependency.