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

Package left un-upgraded with pip install --upgrade #10613

Closed
1 task done
aphedges opened this issue Oct 23, 2021 · 3 comments · Fixed by #10615
Closed
1 task done

Package left un-upgraded with pip install --upgrade #10613

aphedges opened this issue Oct 23, 2021 · 3 comments · Fixed by #10615
Labels
C: dependency resolution About choosing which dependencies to install good first issue A good item for first time contributors to work on type: bug A confirmed bug or unintended behavior

Comments

@aphedges
Copy link
Contributor

aphedges commented Oct 23, 2021

Description

When upgrading both datasets and fsspec in an environment, only datasets was upgraded to the latest version even though there was nothing constraining fsspec from being updated. fsspec is a dependency of datasets (fsspec[http]>=2021.05.0 is specified in its setup.py), and from looking at the resolver's debug output, it seems that pip is preferring to use the pre-installed version of fsspec instead of the newer one.

The documentation for --upgrade says, "Upgrade all specified packages to the newest available version." Unless I'm misunderstanding, all packages explicitly listed on the command line should have been updated if possible.

Expected behavior

fsspec should be upgraded to 2021.10.1.

pip version

21.3.1

Python version

3.7.12

OS

macOS 10.15.7

How to Reproduce

  1. Create a file named pip_fsspec_test.sh with the following contents:
#!/usr/bin/env bash

set -euxo pipefail

# Prepare environment
python --version
pip install -U pip setuptools wheel
pip list

# Install outdated packages
pip install datasets==1.13.3 fsspec==2021.10.0
pip list --outdated

# Attempt to upgrade both packages
pip install -U 'datasets<=1.14.0' 'fsspec<=2021.10.1'
pip list --outdated
  1. Run bash pip_fsspec_test.sh &> pip_fsspec_log.txt in a new environment to create pip_fsspec_log.txt.
  2. Change the penultimate line of the script to PIP_RESOLVER_DEBUG=1 pip -vvv install -U 'datasets<=1.14.0' 'fsspec<=2021.10.1'.
  3. Run bash pip_fsspec_test.sh &> pip_fsspec_verbose_log.txt in a new environment to create pip_fsspec_verbose_log.txt.

Output

See files under "How to Reproduce".

Code of Conduct

@aphedges aphedges added S: needs triage Issues/PRs that need to be triaged type: bug A confirmed bug or unintended behavior labels Oct 23, 2021
@pradyunsg pradyunsg added C: dependency resolution About choosing which dependencies to install and removed S: needs triage Issues/PRs that need to be triaged labels Oct 23, 2021
@pradyunsg
Copy link
Member

That said, looking at the report more closely, we're preferring the installed version of a package if it's specified with an extra, as a dependency of another package -- even when that dependency is explicitly specified on the CLI. That's... suboptimal.

Does --upgrade-strategy=eager do what you want?

@uranusjr
Copy link
Member

uranusjr commented Oct 23, 2021

Yeah it seems like we need the split("[") treatment in

def _eligible_for_upgrade(name: str) -> bool:
"""Are upgrades allowed for this project?
This checks the upgrade strategy, and whether the project was one
that the user specified in the command line, in order to decide
whether we should upgrade if there's a newer version available.
(Note that we don't need access to the `--upgrade` flag, because
an upgrade strategy of "to-satisfy-only" means that `--upgrade`
was not specified).
"""
if self._upgrade_strategy == "eager":
return True
elif self._upgrade_strategy == "only-if-needed":
return name in self._user_requested
return False

It's quite interesting this bug took so long to surface.

@uranusjr uranusjr added the good first issue A good item for first time contributors to work on label Oct 23, 2021
@aphedges
Copy link
Contributor Author

@pradyunsg, I can confirm that your workaround (--upgrade-strategy=eager) works.

@uranusjr, thanks for creating a fix! I can confirm your PR also solves the problem for me.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: dependency resolution About choosing which dependencies to install good first issue A good item for first time contributors to work on type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants