Column formatting for "pip list" #3654

dougthor42 · 2016-05-04T20:52:06Z

Summary

This PR adds optional column formatting to pip list via the commands --columns and --no-header.

Discussion

There is a lot of looping going on to accomplish this PR. I don't think it's a big deal because the list of installed packages is rarely large enough that having multiple loops would cause a significant slowdown. Where reasonable (for speed and readability), I've used list comprehensions instead.

While the call signature of output_package_listing was changed, it was not changed in a way that would effect other calls since the new options argument defaults to None.

If output_package_listing is called with options=None then there is no change to the previous behavior.

Examples

Standard output of pip list is not changed. New output looks like so:

$ pip list --columns
Package        Version
-------------- ------------
docopt         0.6.2
idlex          1.13
jedi           0.9.0
pip            8.1.1
prompt-toolkit 0.60
ptpython       0.32
Pygments       2.1.3
pythonnet      2.1.0
retry          0.8.1
setuptools     20.6.7
six            1.10.0
sqlite-bro     0.8.11
wcwidth        0.1.6
winpython      1.5.20160402

$ pip list --columns -o
Package    Version Latest Type
---------- ------- ------ -----
retry      0.8.1   0.9.1  wheel
setuptools 20.6.7  21.0.0 wheel

$ pip list --columns
Package        Version      Location
-------------- ------------ -----------------------------------
colorama       0.3.7
docopt         0.6.2
idlex          1.13
jedi           0.9.0
pip            8.1.1
pluggy         0.3.1
prompt-toolkit 0.60
ptpython       0.32
py             1.4.31
Pygments       2.1.3
pytest         2.9.1
pythonnet      2.1.0
setuptools     20.6.7
six            1.10.0
sqlite-bro     0.8.11
tox            2.3.1
tqdm           4.5.0        c:\gitlab\temp\path with space\tqdm
virtualenv     15.0.1
wcwidth        0.1.6
winpython      1.5.20160402

$ pip list --no-columns
DEPRECATION: The --no-columns option will be removed in the future.
colorama (0.3.7)
docopt (0.6.2)
idlex (1.13)

This change is

dougthor42 · 2016-05-04T22:10:02Z

Whoops, I forgot unit tests. Working on those now.

As for the failing travis-ci tests:

W503 line break before binary operator: I didn't see any precedence for using line-break-after-binary-operator, so I went with Knuth-style as recommended by PEP8. If you want me to change it to line break after binary operator, I have no issue doing so.
Not sure what to do about TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'. Is that something on my end?

dougthor42 · 2016-05-05T01:29:43Z

I'll continue fixing the unit test failures tomorrow.

Need to figure out (I'm talking to myself here; respond if you'd like but don't feel like it's necessary):

How to assert that pip list --no-header raises CommandError. I'm not used to py.test (I prefer stdlib unittest) and for some reason with pytest.raises(CommandError): script.pip(...) doesn't work.
- do I need to modify PipTestEnvironment.run?
- do I need expect_stderr=False?
How to handle data = [x + [x[0].location] for x in data]; AttributeError: 'str' object has no attribute 'location'
- I could just pass that error, but that seems like bad juju.
- I need to get some dummy outdated packages locally so that I can test things without having to use travis-ci.

dougthor42 · 2016-05-06T22:42:31Z

pip/commands/list.py

        for dist, latest_version, typ in sorted(
                self.find_packages_latest_versions(options),
                key=lambda p: p[0].project_name.lower()):
            if latest_version > dist.parsed_version:
+                latest_pkgs.append((dist, latest_version, typ))
+
+        if (hasattr(options, "columns") and


I'm not a fan of the logic in this block, but it works. 😕

I'm comfortable with "get it working first, tidy up later". So don't worry about it for now.

dougthor42 · 2016-05-07T02:21:02Z

Alrighty, all tests are finally passing.

Couple of questions:

There are a lot of commits in this PR. Do you want me to recreate it with just a commit or two?
- Sorry about that! I was unable to get tests working locally, so I used this and another dummy branch for testing.
Some of the logic isn't as nice as I'd like it to be. Would you like me to continue working on that or just leave it as-is?
- list.py: lines 134-147, 168-188, and 264-289
- lots of extra looping that might be able to be removed
I only added a single unittest for the --columns --no-columns case. Should I add some for the other interactions, such as --outdated --columns --no-columns?

pfmoore · 2016-05-07T10:36:04Z

pip/commands/list.py

@@ -53,7 +58,7 @@ def __init__(self, *args, **kw):
            help=('If in a virtualenv that has global access, do not list '
                  'globally-installed packages.'),
        )
-        self.cmd_opts.add_option(
+        cmd_opts.add_option(
            '--user',


This looks like an unrelated bug-fix? If so, could you split it out into a separate PR?

Not that I'm aware of. I must have removed it during one of my "let's clean things up" sprees. I noticed that this had self while others didn't. I'll revert it.

pfmoore · 2016-05-07T10:59:53Z

Overall, looking good. I added some review comments.

The one big one is that you seem to be generating the --no-columns deprecation warning in the implicit case where the user doesn't supply any option and is getting the current default behaviour. That's not correct, and needs fixing. (Which could be tricky, as simply looking in sys.argv isn't sufficient - the user could have the option set in an environment variable, which would need a warning). Maybe have the default value for the option be a distinguished value (None) then after options have been processed, check the option - if it's False then the user explicitly set it, so warn, if it's None then it's unspecified so set it to False (the current behaviour).

To answer your questions:

Personally, I'd prefer it if you'd squash the commits, but it's not a huge deal.
Again, tidying up the logic is not a huge problem either way to me - if you want to tidy the logic up, feel free, but I'm perfectly OK with having it go in working but a little clumsy, with the option of tidying up later (feel free to include a # TODO: Clean up this logic comment in that case, if you want).
While I haven't reviewed all of the details of the tests you've added, they seem sufficient. If we did coverage checks, I'd say that what matters is did you cover all of the added code. Without those, my instinct is that the sort of extra cases you're suggesting checks for won't improve coverage, so don't worry about it.

dstufft · 2016-05-07T14:50:35Z

I think you should be able to figure out the warning by just having three states on the same variable like this:

# TODO: When we switch the default, set default=True here and then
#              adjust pip/commands/list.pip to remove ``and options.columns is None``.
columns = partial(Option, "--columns", dest="columns", action="store_true")
no_columns = partial(Option, "--no-columns", dest="columns", action="store_false")

I think this should work in such a way that if someone specifies --columns then the options.columns will be True, if the specify --no-columns then it will be False, and if the specify nothing it will be None. Then your deprecation warning can be:

# TODO: Remove the ``and options.columns is None`` when we switch
#              the default.
if not options.columns and options.columns is None:
    warnings.warn(...)

dougthor42 · 2016-05-07T15:57:53Z

generating the --no-columns deprecation warning in the implicit case

Yes, that's what was happening. I guess I misunderstood @dstufft request in #3651 - it sounded like you wanted the deprecation warning in the implicit case.

Alrighty, cool. Thanks for the feedback. I'll be able to run these changes through on Monday (GMT-8).

dougthor42 · 2016-05-07T16:03:56Z

docs/reference/pip_list.rst

+
+ ::
+
+  $ pip list --columns --no-header


--no-header is no longer a valid option (was replaced with --no-columns)

Ivoz · 2016-05-09T05:23:13Z

As a check point, needs an item in the changelog

dougthor42 · 2016-05-09T20:52:05Z

Alrighty, I think we're good to go again.

No deprecation warning shown for the implicit pip list case
Updated changelog
All unit tests passing
Was able to clean up some logic that I didn't like.
Examples added to documentation
Original PR comment updated to reflect final functionality
- removed --no-header example
- added --no-columns example
Commits squashed

Let me know if I'm missing anything.

dstufft · 2016-05-12T10:16:54Z

Hey, this looks pretty good to me and I'd be happy to merge it, but It appears it has merge conflicts. If you get the time to fix that i can merge it then or I'll try to pull it down and rebase it myself. Whichever you prefer.

Thanks a lot!

pfmoore · 2016-05-12T10:20:28Z

Agreed this is looking good. Thanks @dougthor42 for doing this!

xavfernandez · 2016-05-12T14:09:36Z

I like it also but would be more in favor of a --format=columns option that could easily be extended to also add --format=json and so on.

dstufft · 2016-05-12T14:10:23Z

Once the deprecation process is over --no-columns will be gone and --columns will be a no-op (or removed).

xavfernandez · 2016-05-12T14:11:07Z

(and maybe even a --format=freeze to merge pip list and pip freeze)

pfmoore · 2016-05-12T14:32:43Z

@xavfernandez I'm inclined to wait until someone actually implements a JSON or freeze format and at that point, add a --format option. Whether that needs to replace the --columns option largely depends on whether that has been removed yet - but I don't imagine it would be a huge job either way.

Let's keep this PR focused on the one improvement for now.

xavfernandez · 2016-05-12T15:32:57Z

@pfmoore Agreed.
But someone might actually add json/freeze formats saturday morning in its train ;)

pfmoore · 2016-05-12T15:52:12Z

Looking forward to it :-)

+ No changes to current functionality or output. + `pip list --columns` will display items aligned into columns. + Added unit tests for new commands + Updated documentation + Added notes for future deprecation of --no-columns.

dougthor42 · 2016-05-12T16:17:02Z

Looks like the conflict was simply with CHANGES.txt so I've gone ahead and fixed it.

dstufft · 2016-05-12T16:17:37Z

Great! Will merge once tests pass again and the status is green. Thanks a lot :)

'pip list' formatting has changed, and it shows a deprecation warning if we don't set the --format option: pypa/pip#3654

dougthor42 changed the title ~~Column formatting for "pip list"~~ [WIP] Column formatting for "pip list" May 5, 2016

dougthor42 reviewed May 6, 2016
View reviewed changes

dougthor42 changed the title ~~[WIP] Column formatting for "pip list"~~ Column formatting for "pip list" May 7, 2016

dougthor42 mentioned this pull request May 7, 2016

Is there any desire for a "pretty print" output of "pip list"? #3651

Closed

pfmoore reviewed May 7, 2016
View reviewed changes

dougthor42 reviewed May 7, 2016
View reviewed changes

dstufft merged commit 6dcd9da into pypa:develop May 12, 2016

dougthor42 deleted the pip-list-pretty-formatting branch May 12, 2016 18:10

xavfernandez mentioned this pull request May 13, 2016

Add --format option to pip list #3686

Merged

xavfernandez mentioned this pull request Sep 28, 2016

pip list --outdated format like npm #2982

Closed

guewen added a commit to camptocamp/docker-odoo-project that referenced this pull request Nov 18, 2016

Remove pip warning

286dec3

'pip list' formatting has changed, and it shows a deprecation warning if we don't set the --format option: pypa/pip#3654

frdmn mentioned this pull request May 23, 2017

Fix pip deprecation warning Add environment variable to force legacy listing mattly/bork#91

Merged

pfmoore mentioned this pull request Sep 26, 2017

pip list produces error #4087

Closed

lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019

lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Column formatting for "pip list" #3654

Column formatting for "pip list" #3654

dougthor42 commented May 4, 2016 •

edited

Loading

dougthor42 commented May 4, 2016

dougthor42 commented May 5, 2016

dougthor42 May 6, 2016

pfmoore May 7, 2016

dougthor42 commented May 7, 2016

pfmoore May 7, 2016

dougthor42 May 7, 2016

pfmoore commented May 7, 2016

dstufft commented May 7, 2016 •

edited

Loading

dougthor42 commented May 7, 2016

dougthor42 May 7, 2016

Ivoz commented May 9, 2016

dougthor42 commented May 9, 2016

dstufft commented May 12, 2016

pfmoore commented May 12, 2016

xavfernandez commented May 12, 2016

dstufft commented May 12, 2016

xavfernandez commented May 12, 2016

pfmoore commented May 12, 2016

xavfernandez commented May 12, 2016

pfmoore commented May 12, 2016

dougthor42 commented May 12, 2016

dstufft commented May 12, 2016

Column formatting for "pip list" #3654

Column formatting for "pip list" #3654

Conversation

dougthor42 commented May 4, 2016 • edited Loading

Summary

Discussion

Examples

dougthor42 commented May 4, 2016

dougthor42 commented May 5, 2016

dougthor42 May 6, 2016

Choose a reason for hiding this comment

pfmoore May 7, 2016

Choose a reason for hiding this comment

dougthor42 commented May 7, 2016

pfmoore May 7, 2016

Choose a reason for hiding this comment

dougthor42 May 7, 2016

Choose a reason for hiding this comment

pfmoore commented May 7, 2016

dstufft commented May 7, 2016 • edited Loading

dougthor42 commented May 7, 2016

dougthor42 May 7, 2016

Choose a reason for hiding this comment

Ivoz commented May 9, 2016

dougthor42 commented May 9, 2016

dstufft commented May 12, 2016

pfmoore commented May 12, 2016

xavfernandez commented May 12, 2016

dstufft commented May 12, 2016

xavfernandez commented May 12, 2016

pfmoore commented May 12, 2016

xavfernandez commented May 12, 2016

pfmoore commented May 12, 2016

dougthor42 commented May 12, 2016

dstufft commented May 12, 2016

dougthor42 commented May 4, 2016 •

edited

Loading

dstufft commented May 7, 2016 •

edited

Loading