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

Equivalent behaviour to pip install --find-links #1511

Closed
wants to merge 19 commits into from

Conversation

cc-a
Copy link

@cc-a cc-a commented Oct 28, 2019

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code.

The What

This PR partially satisfies #1391. This PR implements a new repository class that reproduces the behaviour of the --find-links flag to pip install. This allows installation of packages from online locations not compliant with Pep 503.

The How

The new class FindLinksRepository is implemented as a subclass of LegacyRepository as the desired behaviours are very similar with only slight changes in expected directory structure. Minor changes were made to LegacyRepository to provide functions that could be overwritten in the subclass. Combined with implementing FilteredPage as a subclass of Page this provides a fairly minimal approach to implementing this new functionality. A few minor associated changes in factory.py and pip_installer.py were required to accommodate the new repository class.

The Testing

The majority of the business logic of FindLinksRepository is derived from LegacyRepository and hence testing of this has not been repeated. Instead testing focuses on making sure that the behaviour of the major public functions of LegacyRepository is recreated with an appropriately structured test set.

A simple high level test can be performed using pytorch as an example. Create the below entry in a pyproject.toml:

[tool.poetry.dependencies]
python = "^3.7"
torch = "1.2.0+cpu"

[[tool.poetry.source]]
name = "pytorch"
url = "https://download.pytorch.org/whl/torch_stable.html"
find_links = true

Then poetry install.

Not done yet

The behaviour of --find-links to install from local paths or file:// urls has not yet been replicated. I will follow up with another PR for this functionality if this one is successful.

@cc-a cc-a mentioned this pull request Oct 31, 2019
@jkyl
Copy link

jkyl commented Dec 3, 2019

Thank you so much for this PR! The PyTorch example mentioned in your write-up is exactly my use case. Right now the issue that this PR addresses is completely blocking me from accomplishing a platform-independent build with PyTorch using Poetry. I hope this is merged ASAP!

@cc-a
Copy link
Author

cc-a commented Dec 3, 2019

Glad to hear it @jkyl I've rebased to clear the merge conflict which had the side benefit of removing the spurious test failures on Windows which should hopefully smooth the process.

@jkyl
Copy link

jkyl commented Dec 11, 2019

@sdispater any chance of a review?

@finswimmer finswimmer added the kind/feature Feature requests/implementations label Dec 23, 2019
@aviramha
Copy link
Contributor

Any news on this matter?

@abn abn requested a review from a team May 26, 2020 19:39
@abn abn self-assigned this May 26, 2020
@abn
Copy link
Member

abn commented Aug 18, 2020

@cc-a can you rebase this with current master resolve conflicts please? Happy to review this.

@cc-a
Copy link
Author

cc-a commented Sep 1, 2020

So as far as I'm concerned this is good to go. I'm unable to reproduce the linting error locally (different isort versions?) so am not sure whats best to do about that.

@Korijn
Copy link

Korijn commented Sep 12, 2020

Please be aware that even if this PR is merged, issue #2543 still blocks a number of pytorch packaging scenarios as well.

@espdev espdev mentioned this pull request Nov 21, 2020
2 tasks
@masonkirchner
Copy link

@abn @cc-a Can we revisit this PR? This functionality would be required for my team to adopt poetry. Happy to help with the merge conflicts, or jump in elsewhere as needed.

@simonrouse9461
Copy link

Any update on this PR?

@PgLoLo
Copy link

PgLoLo commented Sep 21, 2021

Any updates?

@simonrouse9461 simonrouse9461 mentioned this pull request Sep 22, 2021
2 tasks
@Secrus
Copy link
Member

Secrus commented May 21, 2022

Hi, @cc-a are you still interested in bringing this to Poetry? There had been some changes to the codebase, so your PR would probably be a complete do-over.

@Secrus Secrus added area/repo Meta-issues for the repository/forge itself status/waiting-on-response Waiting on response from author labels May 21, 2022
@neersighted
Copy link
Member

This is actually superseded by the new 'simple' repo type that allows installing from a HTML page of links. Thanks for the PR, and please test the new code in master to make sure it works for your use case!

@mboisson
Copy link

This is actually superseded by the new 'simple' repo type that allows installing from a HTML page of links. Thanks for the PR, and please test the new code in master to make sure it works for your use case!

is there any tool to generate such HTML page of links ?

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/repo Meta-issues for the repository/forge itself kind/feature Feature requests/implementations status/waiting-on-response Waiting on response from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.