Skip to content
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

[Feature Request]: Remove strict version pin on pymatgen #1020

Closed
Andrew-S-Rosen opened this issue Jun 1, 2024 · 14 comments
Closed

[Feature Request]: Remove strict version pin on pymatgen #1020

Andrew-S-Rosen opened this issue Jun 1, 2024 · 14 comments

Comments

@Andrew-S-Rosen
Copy link
Member

Andrew-S-Rosen commented Jun 1, 2024

Problem

"pymatgen==2024.4.13",

Why is a strict version of pymatgen pinned here? Any chance of removing this in the next release? It is quite limiting for external packages that use emmet-core as a dependency. :( Not sure who to ping offhand... @tschaume?

Proposed Solution

No version pin, or at the very least a <= pin in the meantime.

Alternatives

No response

@tsmathis
Copy link
Collaborator

tsmathis commented Jun 3, 2024

This is gonna be in @esoteric-ephemera's wheelhouse. We had an issue with pmg related to this PR: #389

@esoteric-ephemera
Copy link
Collaborator

Like Tyler said, this was to prevent further issues with renaming of args in pymatgen that are specified in downstream packages. Both pymatgen.analysis.{diffusion, defects} were affected.

But yeah, this restriction could be modified to pymatgen<=2024.4.13 if we keep the current pymatgen-analysis-diffusion dependence

Another option is installing pymatgen-analysis-diffusion from git, since this includes the fix, and pymatgen>=2024.4.13

@Andrew-S-Rosen
Copy link
Member Author

Both of those options seem quite reasonable to me, although of course the latter would be greatly preferred if this is going to be a medium-to-long-term situation. 🙏

Or we could ask @shyuep to mint a new version of pymatgen-analysis-diffusion if that is the main blocker?

@tschaume
Copy link
Member

tschaume commented Jun 3, 2024

A new version for both pymatgen.analysis.{diffusion,defects} should be released if possible. We unfortunately can't use installs from git branches as dependencies in MP.

@esoteric-ephemera
Copy link
Collaborator

Defects has already had a new release to patch this (2024.5.11), so diffusion is the blocker to upgrading pymatgen

The fix in pymatgen-analysis-diffusion should work with both older and newer pymatgen releases tho, since it does not explicitly name args

@Andrew-S-Rosen
Copy link
Member Author

Linking to my request for a new release of pymatgen-analysis-diffusion: materialsvirtuallab/pymatgen-analysis-diffusion#389 (comment).

@tsmathis
Copy link
Collaborator

tsmathis commented Jul 2, 2024

@Andrew-S-Rosen, have you gotten any communication on movement for a release for pymatgen-analysis-diffusion?

@Andrew-S-Rosen
Copy link
Member Author

Sadly, nothing...

@tsmathis
Copy link
Collaborator

tsmathis commented Jul 2, 2024

all good, we'll reach out

@shyuep
Copy link
Member

shyuep commented Jul 2, 2024

I am confused what release we are referring to. The latest pymatgen-analysis-diffusion is 2024.6.27

@tsmathis
Copy link
Collaborator

tsmathis commented Jul 3, 2024

Ah that's great, we should be good then. Didn't see there were releases last week, thanks!

@Andrew-S-Rosen, we'll test removing the pmg pin!

@Andrew-S-Rosen
Copy link
Member Author

Thanks, all! I also missed it.

@tsmathis
Copy link
Collaborator

@Andrew-S-Rosen (and @utf since you brought up a similar issue), dependency issues have been resolved now. We should be going ahead with a full release soon, but for now please use 0.84.2rc2 for emmet-core where pymatgen is now unpinned.

And feel free to open a new issue if new problems are encountered.

@Andrew-S-Rosen
Copy link
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants