-
Notifications
You must be signed in to change notification settings - Fork 230
Add more dependencies to root JSON file #6189
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
Add more dependencies to root JSON file #6189
Conversation
| description="""Wrapper of WarpX""", | ||
| package_data=package_data, | ||
| install_requires=["numpy", "picmistandard==0.33.0", "periodictable"], | ||
| install_requires=["numpy", f"picmistandard=={picmi_version}", "periodictable"], |
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 the file here should maybe just use requirements.txt instead of documenting the dependencies again...?
ax3l
left a comment
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.
This looks great to me, thank you!
|
I think there is one more problem to fix, both here and in pyAMReX. For release updates we replace the commit hash with the release tag. However, I think this is buggy for dependencies that do not publish monthly releases regularly, e.g., pybind11 or even PICSAR. Replacing a commit hash from the latest weekly update with the latest available release tag may actually bring us backward in the commit history, if the latest available release tag is older than the latest commit. I think we should be doing something different. I don't see major drawbacks in always keeping the version and the commit separate from each other, using the release tag only for the version (e.g., |
|
In view of the upcoming monthly release, this may be worth reviewing. In particular, the outstanding questions in #6189 (comment). |
We generally try to publish our releases only on tags, because otherwise we make the live of downstream package maintainers (spack, conda, etc.) very hard. For pybind11, I always use published releases. Some tags might be from prior months in our dependencies, which is fine. |
dependencies.json
Outdated
| "commit_picsar": "0c329e66010267662a82219f7de7abbd231463f4" | ||
| } | ||
| "commit_picsar": "0c329e66010267662a82219f7de7abbd231463f4", | ||
| "commit_pybind11": "af796d0a99f0cbd9aebb10591257c41a56811cf6", |
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.
How did you end up with this sha exactly?
pybind11 tag v3.0.1 is commit f5fbe867d2d26e4a0a9177a51f6e568868ad3dc8
| if args.all or args.pybind11: | ||
| repo_dict["pybind11"] = {} | ||
| repo_dict["pybind11"]["commit"] = ( | ||
| "https://api.github.com/repos/pybind/pybind11/commits/master" | ||
| ) | ||
| repo_dict["pybind11"]["tags"] = ( | ||
| "https://api.github.com/repos/pybind/pybind11/tags" | ||
| ) |
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.
Note that we do not do a weekly tracking of pybind11. We manually update when we want a newer release.
Most importantly, when we release WarpX we do not depend on the pybind11 development branch (or any dependency as of their development branch).
ax3l
left a comment
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.
The logic here is off for at least pybind11.
Note that we do on release track a tag, not development. We only track (some) dependencies development weekly between releases.
|
I pushed a fix similar to AMReX-Codes/pyamrex#517. |
ax3l
left a comment
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.
Thank you! :)
Add more dependencies to the JSON file in the root directory, to improve automation: