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

Ignore commas in pip show, etc. #5454

Open
GadgetSteve opened this issue May 30, 2018 · 7 comments
Open

Ignore commas in pip show, etc. #5454

GadgetSteve opened this issue May 30, 2018 · 7 comments
Labels
C: cli Command line interface related things (optparse, option grouping etc) C: list/show 'pip list' or 'pip show' state: needs discussion This needs some more discussion type: feature request Request for a new feature

Comments

@GadgetSteve
Copy link
Contributor

What's the problem this feature will solve?
Currently on using pip show on a package with multiple dependencies they are listed comma separated, e.g.:

>pip show mayavi
Name: mayavi
Version: 4.6.0
Summary: UNKNOWN
Home-page: http://docs.enthought.com/mayavi/mayavi/
Author: Prabhu Ramachandran, et. al.
Author-email: prabhu@aero.iitb.ac.in
License: BSD
Location: c:\python36_64\lib\site-packages
Requires: apptools, envisage, numpy, pyface, pygments, traits, traitsui, vtk
Required-by:

But if, for example we wish to check the licence for all of the dependencies so copy the last line to pip show with:>pip show apptools, envisage, numpy, pyface, pygments, traits, traitsui, vtk it fails silently on everything but vtk. This could easily lead the user, especially if they are using a pipe to filter the licence lines and remove duplicates, to draw the wrong conclusion.

Describe the solution you'd like
It the list processing for pip show and other commands were to add a strip of the comma character rather than the relying user understanding what happened an performing a manual edit then the results would be as intuitively expected.

In the real world example above stripping commas from the target list would have the same effect as >pip show apptools envisage numpy pyface pygments traits traitsui vtk

Alternative Solutions

Additional context

@GadgetSteve
Copy link
Contributor Author

Thinking about it many people skip the space after commas, (incorrectly), and this depending on context may or may not cause a problem so I would like to also split on commas as well.

@pradyunsg pradyunsg added the S: needs triage Issues/PRs that need to be triaged label May 31, 2018
@pradyunsg
Copy link
Member

@GadgetSteve Could you provide additional context and reasoning on why doing this sort of filtering outside of pip isn't a good enough option?

@pradyunsg pradyunsg added C: list/show 'pip list' or 'pip show' S: awaiting response Waiting for a response/more information type: feature request Request for a new feature and removed S: needs triage Issues/PRs that need to be triaged labels Jun 4, 2018
@GadgetSteve
Copy link
Contributor Author

@pradyunsg This actually caught me out the other day where I was checking the licences for mayavi as above, I was considering if it would meet our rules at work and I ran: pip show mayavi which gave me the licence information for mayavi License: BSD and a comma separated list of things that will be installed at the same time. apptools, envisage, numpy, pyface, pygments, traits, traitsui, vtk

I added this, (by copy and paste), to the pip show command line thinking that I would later be able to generalize this by piping into grin, or grep, to get all the requires and then do the same to get the set of licences.

To my surprise the output of: pip show mayavi, apptools, envisage, numpy, pyface, pygments, traits, traitsui, vtk was just the details for vtk - the reason that I was surprised was that I was directly using the output from pip as an input to pip.

It is quite reasonable to envisage someone doing along the lines of grabbing the output of pip show to run pip show without expecting to perform intermediate processing especially in these days when companies are getting more open to using open source software at the same time as getting more cautious of licence implications.

I actually thought of 3 possible solutions to this issue:

  1. Simply change pip show to not comma separate the Requires, (and the Required-by), personally I like seeing commas in lists.
  2. Make a change to pip show so that it does not fail silently: "apptools," not found would have been a useful hint.
  3. Try to get pip show changed so that it will accept as input what it provides as output - this seemed the most reasonable to me.

I do have to admit a distinct personal bias in that I believe that tools should except input in the format(s) that they output. To me this minimizes user confusion and the potential for error.

@pradyunsg pradyunsg added S: needs triage Issues/PRs that need to be triaged and removed S: awaiting response Waiting for a response/more information labels Sep 12, 2018
@chrahunt
Copy link
Member

PRs welcome, preferably separate.

Personally I think:

  1. Simply change pip show to not comma separate the Requires, (and the Required-by), personally I like seeing commas in lists.

It is probably as-is because the output is meant to look like metadata which uses commas in lists for some fields. Maybe there would be some backwards-compatibility concern switching to just space-separated?

2. Make a change to pip show so that it does not fail silently: "apptools," not found would have been a useful hint.

I can't think of a reason why these would have been skipped, but it looks like it has been that way since the beginning. Logging warnings to stderr for packages that weren't found, maybe before any output as opposed to inline with the text, would probably be good.

3. Try to get pip show changed so that it will accept as input what it provides as output - this seemed the most reasonable to me.

I think that the proposed approach of splitting and/or dropping commas opens the door for more confusion. There are plenty of people whose focus is not on the command line, or who only need to use one or the other of e.g. cmd, PowerShell, or *sh sparingly. Seeing a comma-separated list of input to a command without familiarity in all these tools the first thought may not be that pip is the one being flexible in what it accepts as input. If you are familiar with the context then it will look like an error on the part of whoever wrote it, since it is not really typical CLI behavior in my experience (but maybe because I haven't tried).

@chrahunt chrahunt added C: cli Command line interface related things (optparse, option grouping etc) state: needs discussion This needs some more discussion labels Aug 10, 2019
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Aug 10, 2019
@GadgetSteve
Copy link
Contributor Author

Added a new issue #6858 to separate the issues, and any PRs, as pointed out above by @chrahunt this is really a separate issue that has been about for a long time.

@GadgetSteve
Copy link
Contributor Author

Note that the proposed change is not to require commas in the input list but rather to allow them - this shouldn't cause as much confusion as reporting package, Not Found where the comma may be seen as a text separator rather than a part of the name. This could be reduced by putting the package name in quotes: "package," Not Found but still requires the user noticing one of the smallest characters going.

Personally I would like to see:
pip show a, b, c
just work as well as:
pip show a, b, c

I can see no reason for the same to be true of pip install or any other command.

Simply changing in pip/_vendor/packaging/utils.py:

def canonicalize_name(name):
    # This is taken from PEP 503.
    return _canonicalize_regex.sub("-", name).lower()

to:

def canonicalize_name(name):
    # This is taken from PEP 503.
    return _canonicalize_regex.sub("-", name).lower().rstrip(',')

Would do the job.
Note that allowing:
pip command a,b,c # No spaces after the commas
Would be a more complex change and I am not currently suggesting this.

@GadgetSteve
Copy link
Contributor Author

Wondering about the scope of the required testing!

  1. Test that the desired change works in the specific expected place OR
  2. Test all of the places that it will result in a change of behavior, i.e. just about every pip command would be able to accept a comma space separated list as well as a space separated list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: cli Command line interface related things (optparse, option grouping etc) C: list/show 'pip list' or 'pip show' state: needs discussion This needs some more discussion type: feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

3 participants