-
Notifications
You must be signed in to change notification settings - Fork 14
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
Introduce the bufr_query library from NOAA-EMC #461
Introduce the bufr_query library from NOAA-EMC #461
Conversation
|
||
# Optional dependencies | ||
variant("python", default=True, description="Enable Python interface") | ||
depends_on("python@3:", type=("build", "run"), when="+python") |
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.
I think that the correct syntax for Python is something like extends(python)
and then use the PythonBuilder to build and install the Python interface. See https://github.com/spack/spack/pull/45504/files for how this is done. I haven't implemented a package this way myself yet, but I understand that this is how it is supposed to be.
At the minimum, please remove @3:
because the minimum version that spack supports is 3.7
.
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.
I grabbed this from another package script too. The example you give looks a lot nicer. I can take a look at that. Thanks!
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.
I switched the code over to use the extend("python") mechanism (as is done in the example PR you noted).
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.
Great that you got this to work so quickly. I haven't tested this myself, but I am happy to approve if this works for you.
modules in the expected "lib/pythonX.Y/site-packages" directory.
Tested this and verified that the bufr_query install is working. This is good to go now. I solved the PYTHONPATH issue with a patch, and at the same time I've submitted a PR to bufr-query with the changes in the patch. |
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.
Approving again. Looks good to me
The commit history is a bit too long for merging this as a regular merge commit (which we usually do for merges into the spack submodule for easier cherry-picking for upstream spack develop). I am going to merge this as a squash merge and we'll have to do something like |
This PR adds in a new package.py script for the new bufr_query library from NOAA-EMC. This is being used by JEDI and other applications.
I have created a PR in the authoritative spack repo with these changes: spack#45920 |
* Introduce the bufr_query library from NOAA-EMC (JCSDA#461) This PR adds in a new package.py script for the new bufr_query library from NOAA-EMC. This is being used by JEDI and other applications. * Add explicit build dependency spec to the pybind11 depends_on spec Co-authored-by: Wouter Deconinck <[email protected]> * Convert patch file to the URL form which pulls the changes from github. Co-authored-by: Wouter Deconinck <[email protected]> * Added new version (0.0.3) and removed obsolete site-packages.patch file --------- Co-authored-by: Wouter Deconinck <[email protected]>
This PR adds in a new package.py script for the new bufr_query library from NOAA-EMC. This is being used by JEDI and other applications.
This partially addresses JCSDA/spack-stack/issues/1182