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

Update pip install pipline #461

Merged
merged 1 commit into from
Sep 9, 2021
Merged

Update pip install pipline #461

merged 1 commit into from
Sep 9, 2021

Conversation

matentzn
Copy link
Contributor

@matentzn matentzn commented Sep 8, 2021

We had huge issues with pip install and the resolver not working anymore properly.

pypa/pip#9187

That was a painful thing to do - not sure though how they will fix this or if its a problem on our end..

@gouttegd
Copy link
Contributor

gouttegd commented Sep 8, 2021

I don’t have much hope a fix will ever come from pypa… In the end we might need to pinpoint what exactly in our dependencies makes the resolver go astray. I’ll try to have a look at that.

----- BEGIN RANT -----
Sigh… Did I ever mention that I hate automatic dependency management? This kind of problems is a perfect illustration why.

There’s a fine line between clever and stupid. Automatic dependency management programs try to be so clever that they wrap the cleverness counter and end up on the stupid side.

----- END RANT -----

@matentzn
Copy link
Contributor Author

matentzn commented Sep 9, 2021

Do you agree with the workaround for now? Any objections, other ideas?

@gouttegd
Copy link
Contributor

gouttegd commented Sep 9, 2021

I will test this morning whether the workaround doesn't break anything on the arm64 (m1) architecture. I don't expect any problem, but you never know…

Longer term, I wonder if maybe we should "lock" the versions of all the Python packages we're using, as suggested here. We find a set of versions for our packages that we know is working well, and we bake that into the ODK. We can then still update those packages from time to time, but at least we are not dependent on the (obviously broken) dependency resolver of pip every time we build the ODK…

@matentzn
Copy link
Contributor Author

matentzn commented Sep 9, 2021

I was thinking the same thing. Locking is considered bad practice for packages, but for docker images like ODK it makes total sense to control the versions of the distributed python packages. In fact, I would even consider this right now. Here is the list:

click==8.0.1
dacite==1.6.0
dataclasses==0.6
dataclasses-json==0.5.5
dataclasses-jsonschema==2.14.1
funowl==0.1.9
Jinja2==3.0.1
jsonpath-rw==1.4.0
jsonschema==3.2.0
jupyter==1.0.0
kgx==1.5.2
linkml==1.0.6
linkml-model==1.0.4
linkml-runtime==1.0.15
matplotlib==2.2.3
matplotlib-venn==0.11.6
mkdocs==1.2.2
networkx==2.3
numpy==1.21.2
ontobio==2.7.9
ontodev-cogs==0.3.3
pandas==1.3.2
plotly==5.3.1
PyGithub==1.55
PyYAML==5.4.1
rdflib==6.0.0
requests==2.22.0
ruamel.yaml==0.17.16
seaborn==0.11.2
sssom==0.3.2
UpSetPlot==0.6.0

Do you believe this to be the better solution?

@gouttegd
Copy link
Contributor

gouttegd commented Sep 9, 2021

Your initial workaround works fine on M1. :)

As for pinning each package to a version, I am not sure it is a better solution but I like it better than forcing the use of a legacy resolver that the pip developers may one day decide to remove from under us (and then we will be forced to address the issue – so I'd rather use a sane set of dependencies now instead of kicking the can down the road).

However your list of versions does not seem to be the "sane set" we are looking for, since rdflib==6.0.0 conflicts with funowl==0.1.9 because the latter requires rdflib~=5.0:

ERROR: Cannot install -r /tools/requirements.txt (line 6) and rdflib==6.0.0 because these package v
ersions have conflicting dependencies.

The conflict is caused by:
     The user requested rdflib==6.0.0
     funowl 0.1.9 depends on rdflib>=5.0.0 and ~=5.0

@matentzn
Copy link
Contributor Author

matentzn commented Sep 9, 2021

Weird how the legacy resolver came up with rdflib 6, while its clearly not allowed :D Another reason for using the fixed dependecies.

@matentzn matentzn merged commit 0394e5d into master Sep 9, 2021
@matentzn matentzn deleted the pip-issues branch September 9, 2021 17:22
@matentzn
Copy link
Contributor Author

matentzn commented Sep 9, 2021

I merged this now as its blocking other stuff..

Made a follow up ticket here #463

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

Successfully merging this pull request may close these issues.

2 participants