-
Notifications
You must be signed in to change notification settings - Fork 45
Introduction of pyecrad #45
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
base: master
Are you sure you want to change the base?
Conversation
3ab6971 to
2f258ac
Compare
|
The python package can be installed in two ways:
The wheels are built and pushed on pypi by A simple test can be performed with |
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.
Many thanks for this, that looks very interesting. I would like to give that a proper round of testing before I will be able to make more substantial remarks on the implementation. In the meantime, I'm flagging already a few things that would help the integration:
- Would you mind adding a
READMEto thepyecraddirectory to explain the purpose and use? - Can you please add licence headers to the script and Python files? Take a look at the equivalent headers in other files for this.
- We will need a build system integration into CMake as well. We have templates for this e.g. in https://github.com/ecmwf-ifs/ifsbench and https://github.com/ecmwf-ifs/loki that we can adapt here. I can help with that.
- I would like to add the definition of some CTest tests (e.g. via pytest) to ensure this remains functional going forward, e.g., by comparing the produced output against the equivalent output of CTest runs of the pure Fortran binaries. Again, I can help with that.
- The
compare.pyscript serves the same purpose as the existing nccmp.py script, could the latter be used instead of duplicating the functionality?
I hope that makes sense and happy to help with the above where required.
This PR aims at introducing a python wrapper around ecrad.