monarch-initiative / mondo-ingest

Coordinating the mondo-ingest with external sources
https://monarch-initiative.github.io/mondo-ingest/
6 stars 3 forks source link

Python: Do we always want the most updated version of requirements? Or just some? #57

Open joeflack4 opened 2 years ago

joeflack4 commented 2 years ago

Summary

I recently added a .PHONY make target called python-install-dependencies. It upgrades to the latest pip, but I didn't think through things enough, and I didn't realize that my local Docker container will install the latest versions of these requirements the first time, but in the future it will not upgrade them; it'll use whatever version they were the first time this was ran.

I put a bug label, because this is frequently going to cause issues when I look up OAK or SSSOM documentation and use things that are in a later version than we have.

Recent changes

Before:

python-install-dependencies:
    python3 -m pip install --upgrade pip
    python3 -m pip install  -r $(RELEASEDIR)/requirements-unlocked.txt

Now:

python-install-dependencies:
    python3 -m pip install --upgrade pip
    python3 -m pip install --upgrade -r $(RELEASEDIR)/requirements-unlocked.txt

I added --upgrade. Now it will install the latest versions of these dependencies: requirements-unlocked.txt:

jinja2
oaklib
pandas
pyyaml
sssom

Possible solution

The requirements-unlocked.txt file that I have is my way of dealing with handling locked/unlocked dependency versions using vanilla pip. However, requirements.txt resolves all transitive dependencies and sets specific versions, as I'm sure you know:

requirements.txt:

aiohttp==3.8.1
aiosignal==1.2.0
alabaster==0.7.12
# ... and so on

The most stable thing to do, probably, is install from requirements.txt, and then upgrade the latest (hopefully stable) from specific libraries, and then update requirments.txt with the new changes.

Here's what I'm thinking:

python-install-and-update-dependencies:
    python3 -m pip install --upgrade pip
    python3 -m pip install -r $(RELEASEDIR)/requirements.txt
    python3 -m pip install --upgrade oaklib sssom
        pip freeze > requirements.txt

@matentzn What do you think? I might just go ahead and make these changes now and revert them if you have something different in mind.

matentzn commented 2 years ago

I dont have any better idea to be honest. I would still prefer using fixed locked dependencies so that the code does not suddenly break during a future run where some dependency introduced a breaking change though

joeflack4 commented 2 years ago

I think that makes a lot of sense.

My only worry is that I know myself, and I know I'll run into bugs and my first reaction will not always be to check if I have the latest version of the dependency installed.

Basically that is what's going to end up being what drives our versioning-up of dependencies in mondo-ingest. Encountering bugs, finding out that these libs are outdated, and then updating them.

It would be good to know when a new version is released, but even then, I wouldn't have time to fully evaluate. And it's really hard to tell if a new version might have a bug / breaking change in it without using it first.


Perhaps the best thing to do would be, if the exception is being raised from one of these dependencies, e.g. oaklib or sssom, to then check to see if there's a new version, and print out a message about that. But that's probably too much work.

I suppose I'll come to a decision on this on my own and then come back and close the issue.

matentzn commented 2 years ago

Yeah, not clear cut for me either.. Hopefully these libraries stablise at some point. Mayeb better to just add the commands into the makefile so at least when you are hacking the makefile you see which versions are being installed..