Skip to content

Poetry remove should not need -d argument #2058

Closed as not planned
Closed as not planned
Poetry remove should not need -d argument#2058
Labels
area/uxFeatures and improvements related to the user experiencekind/featureFeature requests/implementations

Description

@caleb15
caleb15
opened on Feb 19, 2020
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • I have searched the documentation and believe that my question is not covered.

Feature Request

I was very surprised to find out I need to specify -d to remove dev dependencies. Unless there's a reason I'm unaware of, removing a package should remove it regardless of whether it is a dev dependency or regular dependency.

Implementing this would bring poetry in line with other package managers like npm or pip that allow you to remove packages regardless of their status.

Activity

  1. caleb15
    added
    kind/featureFeature requests/implementations
    on Feb 19, 2020

  2. mathieucaroff commented on Apr 14, 2020

    @mathieucaroff
    mathieucaroff
    on Apr 14, 2020

    I think the stakes here are mostly about the principle of least surprise.

    Allowing to uninstall packages without the --dev argument would reduce the surprise for developers coming from npm, pip and yarn, however, poetry should keep the --dev flag for retro-compatibility. But this means we have a useless --dev flag for remove, which is a surprise. -- Note however, that none of the surprises are dangerous, in the sense that none of them "defies expectations".

    In my opinion, the interface of poetry should stay the same. However, when a user tries to uninstall a dev dependency without --dev, poetry should tell them the correct command line to remove the dependency.

    Currently:

    $ poetry remove autopep8 yapf

[ValueError]
Package autopep8 not found

    It'd be nice to have a hint:

    $ poetry remove autopep8 yapf

[ValueError]
Package autopep8 not found

autopep8 is installed as a developer dependency.
Use `poetry remove --dev autopep8` to remove it.

  3. caleb15 commented on Apr 14, 2020

    @caleb15
    caleb15
    on Apr 14, 2020
    Author

    But this means we have a useless --dev flag for remove, which is a surprise

    A useless flag is less surprising than a required flag. A required flag means people have to use it, which means more surprises. A useless flag is only a surprise if looking through the documentation. After a while of it being deprecated it can be removed.

    It's also easier for the developer if they don't have to specify --dev.

  4. caleb15 commented on Apr 14, 2020

    @caleb15
    caleb15
    on Apr 14, 2020
    Author

    Actually let me restate, I don't feel like I said that clearly. Following the principle of least suprise, it would be better to not require the --dev flag.

    Requiring the --dev flag: People are very surprised. Regardless of whether you have a message telling them what to do, there's still the initial surprise and a different behavior that they have to remember.

    Not requiring: People are only slightly surprised when looking through the documentation

  5. mathieucaroff commented on Apr 14, 2020

    @mathieucaroff
    mathieucaroff
    on Apr 14, 2020

    Your least-surprise quantification argument actually convinced me.

    I wonder if we should aim to remove the --dev flag from the CLI, or keep it to reduce friction.

    After pondering a bit, I think we should aim to remove it.

    Side note:
    I just checked what how yarn remove did it and I figured yarn (1.22.0) simply ignores all unknown flags (>~<).

  6. bhaveshpraveen commented on May 8, 2020

    @bhaveshpraveen
    bhaveshpraveen
    on May 8, 2020

    I'd like to work on this issue.

  7. finswimmer
    added
    area/uxFeatures and improvements related to the user experience
    on Oct 14, 2020

  8. abn commented on Oct 14, 2020

    @abn
    abn
    on Oct 14, 2020
    Member

    @bhaveshpraveen were you still thinking of working on this?

  9. bhaveshpraveen commented on Oct 14, 2020

    @bhaveshpraveen
    bhaveshpraveen
    on Oct 14, 2020

    @abn I did not receive any response so I didn't pick it up then. If it's alright, I'd like to pick this up now. I'll be able to work on this over the weekend.

  10. silverwind commented on Oct 14, 2020

    @silverwind
    silverwind
    on Oct 14, 2020

    Here's my thoughts. I see two options to resolve:

    1. Ignore the --dev argument on remove like other package managers and remove the dependency from both types. This is my preference because when removing a package, I do not want to think about where it was defined. If the same package is defined in both, remove it in both.

    2. Provide a more helpful warning or a prompt when a package is in another category then derived from the --dev argument. Not my preference, but better then the current "not found" error.

  11. abn commented on Oct 14, 2020

    @abn
    abn
    on Oct 14, 2020
    Member

    @bhaveshpraveen sure go for it; as discussed here and elsewhere, the expected solution is similar to (2) as described by @silverwind.

    1. If --dev is provided, only attempt to remove development dependency.
    2. If --dev is not specified and
      a. dependency does not exist in in main, but exists in dev raise a confirmation prompt. Note that the prompt must only be a warning if non-interactive mode is used. (@finswimmer we could also just remove this without a prompt since there is no ambiguity here, unsure about when it exists in both - if removing both might upset existing use cases)
      b. dependency exists in main, only remove from main.

    The aspect to remember is that a dependency can existin in both dev and main categories at the same tiem. A user must be able to operate on these individually - this means remove main and remove dev independently if both are present.

  12. mathieucaroff commented on Oct 16, 2020

    @mathieucaroff
    mathieucaroff
    on Oct 16, 2020

    I like @silverwind's Ignore solution, (1. Ignore the --dev argument on remove), because I believe that the scenario of editing a command and not taking the time to remove the --dev option is the most common.

    I do not see the point of installing a package both as a dependency and a dev-dependencies. All dependencies are also dev-dependencies since a developer needs the dependencies to test the code. As such, I believe the support for the use case of having a package installed both as a dependency and a dev-dependency ("installed as both") should be extremely limited. Actually, it is quite unfortunate that poetry supports adding a package both as dependency and as dev-dependency (does it?) ("adding as both").

  13. silverwind commented on Oct 17, 2020

    @silverwind
    silverwind
    on Oct 17, 2020

    Actually, it is quite unfortunate that poetry supports adding a package both as dependency and as dev-dependency (does it?)

    It will add to both but I think only the last added one is the one that will be the active one. Adding a package twice is always an error, site-packages does not support multiple versions of one package so it's always a error.

    the expected solution is similar to (2)

    I'd say the general user expectance is option 1 and that's how prior arts like pip, yarn, npm work too.

  14. finswimmer commented on Oct 17, 2020

    @finswimmer
    finswimmer
    on Oct 17, 2020
    Member

    We cannot ignore the --dev parameter for poetry remove. As @abn said, poetry supports that one have a dependency in both section.

    A use case for this would be for example, you are writing a package, which is a wrapper around black. So you define black as a dependency. At the same time, you want to pin black for a specific version during development to format your code. So you define black as a dev dependency with the pinned version.

    While this wouldn't make any difference during development (poetry will always install the pinned dependency), it makes a different when you build a wheel/sdist which get's installed somewhere else.

    So I agree with @abn suggestions. If --dev is provided, only remove from dev. If --dev is not provided, remove from any section if it's only in one section, otherwise raise an exception or ask the user if they want to completely remove the package.

    fin swimmer

  15. bhaveshpraveen commented on Oct 17, 2020

    @bhaveshpraveen
    bhaveshpraveen
    on Oct 17, 2020

    @finswimmer @abn

    To summarize

    1. If --dev is provided, remove only from dev section.
    2. If --dev is not provided and dependency is present in only one section, then remove it from that section.
    3. If --dev is not provided and dependency is present in both the sections, raise an Exception

    Is that correct?

  16. finswimmer commented on Oct 17, 2020

    @finswimmer
    finswimmer
    on Oct 17, 2020
    Member

    If --dev is not provided and dependency is present in both the sections, raise an Exception

    Now that I read it like this, we should maybe ask the user, from which section the dependency should be removed (dev, non-dev, both). Otherwise we end up in a situation, where we cannot remove a dependency at all.

  17. bhaveshpraveen commented on Oct 17, 2020

    @bhaveshpraveen
    bhaveshpraveen
    on Oct 17, 2020

    Ok, got it.

    Just a thought, I was thinking maybe we could add a separate flag, say something like --all or --both, that removes the dependency from both the sections. This way the user can remove the dependency without having to depend on the prompt(interactive input) from poetry.

  18. bhaveshpraveen
    added a commit that references this issue on Oct 19, 2020

    fixes issue python-poetry#2058

    53420bd

  19. bhaveshpraveen commented on Oct 19, 2020

    @bhaveshpraveen
    bhaveshpraveen
    on Oct 19, 2020

    @abn @finswimmer

    Hey, i've coded it up. I couldn't find any tests for remove operation (Maybe I missed it?) To add tests, is there any snippet/example that I can use for reference? And do I have to make changes in the documentation as well?

  20. caleb15 commented on Apr 1, 2021

    @caleb15
    caleb15
    on Apr 1, 2021
    Author

    @abn @finswimmer following up on above question

  21. finswimmer commented on Oct 5, 2022

    @finswimmer
    finswimmer
    on Oct 5, 2022
    Member

    Now that Poetry have dependency groups, removing a dependency from a group without explicit naming the group, is more surprising for the users. That's why I close this feature request.

  22. finswimmer
    closed this as not plannedon Oct 5, 2022

  23. github-actions commented on Mar 1, 2024

    @github-actions
    github-actionsBot
    on Mar 1, 2024 – with GitHub Actions

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

  24. github-actions
    locked as resolved and limited conversation to collaborators on Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/uxFeatures and improvements related to the user experiencekind/featureFeature requests/implementations

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions