search

materialsproject / api

New API client for the Materials Project
https://materialsproject.github.io/api/
Other
105 stars 33 forks source link

Bug: fatal incompatibility with `pymatgen==2024.2.20` #887

Closed ml-evs closed 3 months ago

ml-evs commented 4 months ago

Problem

As posted here: https://github.com/materialsproject/pymatgen/issues/3644

Hopefully this gets fixed soon-ish but you will never be able to use mp-api with pymatgen==2024.2.20 unless the user also has ASE installed.

Proposed Solution

I assume you don't want to maintain a pymatgen pin but probably a very aggressive CI that checks new releases directly in the MP ecosystem is required to avoid pushing this bother on users/downstream devs.

Alternatives

No response

munrojm commented 4 months ago

Okay, this is good to know thanks for flagging. We do compatibility checks to write out the specific requirements files in requirements/, but don't currently enforce any pinning unless issues like this come up. There has been talk about releasing tested "mp stack" reccomendations with all of the relevant codes but that hasn't come to fruition yet. I'm happy to pin pymatgen explicitly for now until this is resolved.

ThomasWarford commented 4 months ago

I'm having this issue with a fresh install too

JaGeo commented 4 months ago

Yeah, people in my group also had issues today.

munrojm commented 4 months ago

Pymatgen has been pinned in the latest client release to < 2024.2.20.

munrojm commented 4 months ago

Actually, I will leave this thread open until the pmg issue is resolved.

ml-evs commented 4 months ago

I fixed it in https://github.com/materialsproject/pymatgen/pull/3645 so if anyone has review powers on pymatgen feel free to take a look.

There's also https://github.com/materialsproject/pymatgen/pull/3646 which will hopefully prevent this thing from happening in the future by testing with just mandatory deps first (though not sure if this is desirable for the devs yet).

utf commented 4 months ago

The issue in pymatgen has now been resolved by @ml-evs. Is it possible to release a new version of mp-api which un-pins pymatgen?

ml-evs commented 4 months ago

I wonder if we can be more optimistic in future (for tightly coupled packages [in terms of downstream users and developer pool] like these) and add pins like e.g., pymatgen >= 2024, != 2024.2.23 rather than <2024.2.23 to reduce maintenance burden here and elsewhere?

ml-evs commented 4 months ago

Just opened #892 to take advantage of timezones and check that the latest release does pass tests (although not sure if there is a test run that uses the latest compatible versions of all deps, or if they all use pinned requirements/).

munrojm commented 4 months ago

I wonder if we can be more optimistic in future (for tightly coupled packages [in terms of downstream users and developer pool] like these) and add pins like e.g., pymatgen >= 2024, != 2024.2.23 rather than <2024.2.23 to reduce maintenance burden here and elsewhere?

Yup, we should definitely do that.

munrojm commented 4 months ago

Just opened #892 to take advantage of timezones and check that the latest release does pass tests (although not sure if there is a test run that uses the latest compatible versions of all deps, or if they all use pinned requirements/).

They currently use the pinned requirement files. I will unpin and update those now before releasing.