python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.23k stars 2.26k forks source link

provide a simple way to force reinstalling local dependencies #7349

Open trondhindenes opened 1 year ago

trondhindenes commented 1 year ago

Feature Request

Given a pyproject.toml looking like this:

[tool.poetry.dependencies]
some-other-lib = {path = "../other_lib", develop=false}

It seems to me that poetry doesn't provide a way for me to "force reinstall" some-other-lib. If I for example have changes in that lib, how would I get those changes into my current poetry project's virtualenv?

I can remove+install, or bump the version of that lib - but that's of course cumbersome. It would be awesome if poetry provided something like poetry update some-other-lib --force

toaster-code commented 1 year ago

+1 to that feature!

damymetzke commented 5 months ago

I've ran into this just now, specifically with me adding an additional script to a path dependency. I'm willing to submit a patch for this. Currently I'm considering the following:

The problem here is that path dependencies may change in ways that aren't possible for typical packages. So adding a flag like "--reinstall-path-dependencies" would be most appropriate. This would then reinstall all path dependencies. While it would be possible to support manually selecting individual dependencies, personally I don't think it's worth complicating the interface for local operations which should be fast enough in most cases. But I may be corrected on that.

One question is whether or not to support this for transient path dependencies. I haven't considered it enough yet to know how difficult that would be. I'm willing to add that as well, but it may increase how long it will take to implement.

I'm willing to write a patch, probably next week but maybe in 2 weeks. I've never contributed to poetry, can I get some sort of confirmation that it's alright to start a pull request? Or should this feature be discussed further. I'm not aware of the typical process.

radoering commented 5 months ago

I don't have a strong opinion if we need such a feature and if it should be some explicit update --force or --reinstall-path-dependencies so waiting for opinions from other maintainers might make sense.

A few remarks (without being able to say for sure that we would accept such a feature):

The problem here is that path dependencies may change in ways that aren't possible for typical packages.

Is it about path dependencies, i.e. file and directory dependencies or just directory dependencies?

So adding a flag

To which commands? poetry install? poetry update? poetry add?

One question is whether or not to support this for transient path dependencies. I haven't considered it enough yet to know how difficult that would be.

I assume it's easier to consider all path dependencies than to distinguish between direct and transitive dependencies.

I've never contributed to poetry, can I get some sort of confirmation that it's alright to start a pull request? Or should this feature be discussed further. I'm not aware of the typical process.

I don't think there is the one correct way. It's fine to start a pull request if you don't expect it to be merged in any case. Sometimes it's easier to talk about a feature if there is a draft. On the other side, if you want to avoid "unnecessary" work and reduce the risk that your PR is not accepted you can try to discuss the feature further before starting the implementation.

damymetzke commented 5 months ago

Is it about path dependencies, i.e. file and directory dependencies or just directory dependencies?

Personally I've only worked with directory dependencies in this context, and I'm not personally aware of any workflows that would use files instead. Although I suppose the reasoning would end up working the same either way. So I would implement both to stay consistent, but directory only can work as well.

To which commands? poetry install? poetry update? poetry add?

Just adding it to the update makes the most sense to me, I'm not seeing the use for the other commands. And it aligns with the original suggestion.

For now I'll wait for potentially more feedback before committing to writing the patch. I may change my mind some time later though.