-
Notifications
You must be signed in to change notification settings - Fork 11
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
Just update all the dependencies #872
Conversation
Ok, the unit tests fail, but in the usual way with the pydantic issue. Disturbingly, neither nofiles nor tinybase show up in the failures. I took a very brief peek at the source code, and for nofiles I guess this is not surprising -- the only test there is for lammps, so I don't think the tests ever import the problematic |
Instead of trying to close off this chain and rely on the dependabot, what about cutting to the chase and updating |
I'll have a look next week, but unless the import location changed or |
Ah, I see, the import path does change. Should be a quick fix then. |
That is what I usually do for pyiron_base and pyiron_atomistics. I release a new version of base, immediately make the conda-forge PR and merge it, and update pyiron_atomistics to this version 1-2h later once the new version is ready. |
Summary
So The trouble is that we are somehow winding up with For the life of me, I can't figure out what's causing us to get stuck on Full process
This error is coming from where we touch pyiron_atomistics, so why does atomistics not have a similar problem?? Ok, so pyiron_atomistics directly specifies that we need Indeed, moving from mp-api 0.36.1 to 0.37.1 changes the import Neither the pydantic 1.8.2 setup file nor requirements, nor those for 1.10.13 make any reference to pydantic-settings. It does appear in the toml file for the latest v2 of pydantic. Is it possible that mp-api is just missing an explicit dependency on Indeed, I'm actually a little suspicious that the latest version of mp-api is the problem. Their lower-bound of pydantic 1.8.2 was released on 11 May 2021, but the PR introducing pydantic-settings didn't get merged until 7 September 2022. Aha aha, but they do require their package This brings us back to our pip-check failure here:
And sure enough, if I look at the setup for 0.68.0 it's missing So, why is emmet-core not the latest and greatest? I don't find "emmet" mentioned directly anywhere in the pyiron repository. Since it's a materialsproject package, I searched to see if there is another of their projects specifying If we knew who was sticking us with |
Well that was a rabbit hole and a half. Now I'm going to shotgun @jan-janssen @pmrv and @niklassiemer to ask if any of you have any idea where we might be (indirectly) specifying an |
The |
To me |
Yeah, my big concern is I can't figure out why we're getting the downgraded versions of these packages-- where else is the lower requirement coming from? |
I was being a bit daft when I first read your comment -- you're explicitly explaining where the |
The |
I thought to check the github dependencies but these are only direct dependencies, and no transitive search is available on the web portal. Following the advice on the second link I tried using pycharm to search for I'm going to try updating pyiron contrib in my conda env and repeating the pycharm search... |
aimsgb explicitly request |
The strange part is by explicitly pinning |
Crazy. I mean, it makes perfect sense that the |
@jan-janssen, the most recent feedstock for pyiron_atomistics actually has a pretty out-of-date requirement for Do you know why the feedstock would regress the dependency like that? I double-checked and 0.3.3 is indeed the live version on conda-forge right now, so I would have expected the github env yaml versions for that tag to be a floor for the feedstock recipe. |
I created a pull request to fix this issue conda-forge/pyiron_atomistics-feedstock#116 . @niklassiemer did the last releases for |
🚀 Thanks!
Yeah, that's not inconsistent with my just-updated conda env, which wound up giving me I'm a little bit concerned that the feedstock bump to However, if I move the github tag back to 0.36.1, the pyproject.toml requirement drops all the way down to 0.54.0, which is not enough to get us Now, The conda-forge feedstock doesn't contain easy-to-browse tags for past versions, but just looking at the history I can see that as recently as 0.37.0, the emmet-core dependency in the recipe is only >=0.54.0. So my understainding is that if we really want to force conda to get emmet-core 0.69. or be unable to solve, we're going to need |
Of course, none of this explains why we're getting |
It isn't anymore!!!! I asked dependabot to recreate the PR and now it's correctly getting
|
Yes pyiron_atomistics is often taken care of by the conda-forge bot, which, however does not update dependencies... maybe we disable the automerge again? Then one would be forced to look at the PR and update things... |
|
|
|
And add it to setup.py as this package does also appear to be available on pypi?
|
From what I can tell, the heart of the issue is that |
This should be fixed with conda-forge/aws-sam-translator-feedstock#88 |
Awesome! I will anyhow proceed with the extraction of the workflow stuff to its own repo, since that was planned, but this means that in the medium term it will stay possible to have an env that supports both the most recent |
(FYI: right now I intend to remove the workflow stuff from contrib in a separate PR, and after that's working I think we can just close this PR without merging.) |
There was an old lady who swallowed a fly...
pyiron_base can't update until we update pympipool (#867), pyiron_atomistics can't update until we update base (#868), and pyiron_atomistics being out of date is causing our daily tests to fail because we import
StructureFactory
which importsMaterialsProjectFactory
and then the outdatedmp_api
has some conflict with some other dependency on pydantic.However, as discussed in more detail here, I fully expect the major version bump to cause failures where
pyiron_contrib.nofiles
andpyiron_contrib.tinybase
rely onpympipool
-- if this PR passes tests, it will only be because the tests weren't good enough!@jan-janssen and @pmrv, nofiles and tinybase are owned by you, respectively; could you please look into updating them to
pympipool 0.7.0
syntax so we can close off this dependency update chain?