Skip to content

Commit

Permalink
Remove constraints from GitHub in constraints.txt (#2364)
Browse files Browse the repository at this point in the history
#2267 added `torch_harmonics` via a GitHub URL to the
`pip-requirements.txt` file, which subsequently led to it being used in
the `constraints.txt` file. This broke the dataflow image build as newer
versions of `pip` no longer allow this: #2304.

This PR implements the fix suggested in a discussion between @frodre and
@oliverwm1 in Slack a while back:

> Could we just avoid having it in the constraints entirely? It would
still get installed in the image right?

Indeed it seems like even if older versions of `pip` allow GitHub URLs
to appear in constraints files, they do not have the desired effect. In
other words if one does:

```
$ pip install -c constraints.txt torch_harmonics
```

in an environment without it already installed, `pip` will download the
latest version off of PyPI instead of fetching it from the GitHub URL in
the constraints file (perhaps this is not surprising since it is unnamed
and `pip` is not set up to infer the name from the URL). Thus I suppose
it is somewhat misleading to include GitHub URLs in the constraints to
begin with, and sufficient / better that we specify `torch_harmonics`
from GitHub in the `setup.py` in `xtorch_harmonics`:


https://github.com/ai2cm/fv3net/blob/314edd50a1d9f1fe31d14a418dbe981d16c63f1d/external/xtorch_harmonics/setup.py#L16


All that being said, I have implemented this in a way such to retain
`git+https://github.com/NVIDIA/torch-harmonics.git@8826246cacf6c37b600cdd63fde210815ba238fd`
in `pip-requirements.txt` since it appears `pip-compile` can still
resolve dependencies from packages on GitHub and use that to inform the
versions of other packages pinned in the `constraints.txt` file, which
is still useful.

Requirement changes:
- [x] Ran `make lock_deps/lock_pip` following these
[instructions](https://vulcanclimatemodeling.com/docs/fv3net/dependency_management.html#dependency-management)

Resolves #2304

Coverage reports (updated automatically):

---------

Co-authored-by: Oliver Watt-Meyer <[email protected]>
  • Loading branch information
spencerkclark and oliverwm1 authored Nov 6, 2023
1 parent 314edd5 commit 0de3108
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,9 @@ constraints.txt: $(REQUIREMENTS)
.dataflow-versions.txt \
$^ \
--output-file constraints.txt
# remove GitHub links from constraints, since pip does not support unnamed
# constraints, despite the fact that pip-compile includes them:
sed -i '/git+https/d' constraints.txt
# remove extras in name: e.g. apache-beam[gcp] --> apache-beam
sed -i.bak 's/\[.*\]//g' constraints.txt
rm -f constraints.txt.bak .dataflow-versions.txt
Expand Down
1 change: 0 additions & 1 deletion constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,6 @@ tf-estimator-nightly==2.8.0.dev2021122109
threadpoolctl==3.1.0
toml==0.10.1
toolz==0.10.0
git+https://github.com/NVIDIA/torch-harmonics.git@8826246cacf6c37b600cdd63fde210815ba238fd
torch==1.12.1
torchvision==0.13.1
tornado==6.1
Expand Down

0 comments on commit 0de3108

Please sign in to comment.