-
Notifications
You must be signed in to change notification settings - Fork 15
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
Trouble with mp-api in our dependencies #1182
Comments
In terms of my influence, I guess I am known in the materials science community for maintaining the conda-forge packages, but I do not think this ranks my feedback much higher than general users feedback. So I would suggest you open an issue first and if there is no reaction I can try to get more attention to that particular issue, by either emphasising that the issue also affects the conda-forge packages or by contacting the people I know inside the materials project to reach out to the maintainer of the specific package. Still before we do so, we should clarify where the error is coming from and what we can do to prevent this error:
In summary, I am not sure if the While this might sound like a reasonable explanation for what happened, it took me approximately a week of work and over five years of contributing packages to conda-forge to solve this issue and understand what exactly went wrong. So managing dependencies is complex and the easiest way to do it, is to handle one update at a time rather than multiple at once. |
Sounds good, I'll politely suggest they introduce a lower-bound.
that sounds like a very plausible answer to (1)! Doesn't nail it down in details, but at least the gist of things makes sense.
Super, I really like pushing the mp-api requirement as far upstream as possible (namely structuretoolkit). In that case, even if we need to (hopefully just transiently!) pin maggma, it can be done in exactly one spot. In the long run, hopefully the wider ecosystem completing updates makes such a pin completely unnecessary.
💯 I hear you! I appreciate all the work you do maintaining the various packages -- e.g. I didn't even realize you were the maintainer for owlready2! It took me three days just to pin down where the breaking points were with aws-sam-translator + pydantic and mp-api + maggma, much less come up with any real solution; I would be the last to assert dependency management is a simple process! Using greyskull to reduce the points of human interference sounds good to me. |
We now include
mp-api
in our dependency stack to support a structure factory. I ran into trouble where downstream, in packages that depend onpyiron_atomistics
, I got errors that a certain method in themaggma
package (also managed by materialsproject) couldn't be found. The underlying problem was that the CI was installing a too-old version ofmaggma
that doesn't meet the API expectations ofmp-api
, and this is possible becausemp-api
doesn't pin itsmaggma
version at all.It's not currently clear to me why an older version of
maggma
is getting installed. In one case, I was able to stop this from happening by removing dependence onpympipool
; in another case I was able to stop this from happening by simply pinning the correct version ofmaggma
myself (merged version without the owlready complication here). In the latter case, if I can simply pin the higher version and everything works fine, it's completely bizarre to me that conda wasn't just using that version to start with!So, I'm left with two issues:
maggma
when the most recent one works perfectly fine?ironflow
and the newpyiron_workflow
, but in principle such a patch can/should be done here instead. I don't like this as a long term fix -- we don't directly depend onmaggma
and it shouldn't be us ensuring the API is the right one.@jan-janssen you maintain the
mp-api
conda feedstock, does that mean you have a sufficient relationship with the materialsproject folks that a criticism of their versioning practices would be well received compared to such criticism coming from a random stranger (i.e. me)? Because I am academically interested in knowing why the CI installed the old version, but the best technological solution is just for their package to demand the dependencies it needs.The text was updated successfully, but these errors were encountered: