search

materialsproject / emmet

Be a master builder of databases of material properties. Avoid the Kragle.
https://materialsproject.github.io/emmet/
Other
49 stars 63 forks source link

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

Open Andrew-S-Rosen opened 1 month ago

Andrew-S-Rosen commented 1 month ago

Problem

https://github.com/materialsproject/emmet/blob/b8cf8971a8d715afb5ad7a9f7680125d19e86e93/emmet-core/setup.py#L30

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 commented 1 month ago

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

esoteric-ephemera commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Linking to my request for a new release of pymatgen-analysis-diffusion: https://github.com/materialsvirtuallab/pymatgen-analysis-diffusion/pull/389#issuecomment-2146606710.

tsmathis commented 2 days ago

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

Andrew-S-Rosen commented 2 days ago

Sadly, nothing...

tsmathis commented 2 days ago

all good, we'll reach out

shyuep commented 2 days ago

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

tsmathis commented 2 days ago

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 commented 2 days ago

Thanks, all! I also missed it.