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

Freeze package listing in case insensitive order #1605

Merged
merged 3 commits into from
Mar 8, 2014
Merged

Conversation

Ivoz
Copy link
Contributor

@Ivoz Ivoz commented Feb 28, 2014

Fixes #1602

This is also consistent with pip list's behaviour.

@qwcode
Copy link
Contributor

qwcode commented Feb 28, 2014

this makes sense to me, but looks like some tests are order sensitive.
can you confirm pip freeze -r still should work as expected. is there a test for that?

@Ivoz
Copy link
Contributor Author

Ivoz commented Feb 28, 2014

can you confirm pip freeze -r still should work as expected.

Looks fine to me.

The test is partly failing because freeze is spitting out argparse for 2.6

I could try ordering it so editables are always first

(tmp-447ad33f147bb98)ivo@ivosung tmp-447ad33f147bb98$ cat r.txt 
pipa==0.1.0
pyOpenSSL==0.13.1
MarkupSafe==0.18
CherryPy==3.2.4
Jinja2==2.7.2
(tmp-447ad33f147bb98)ivo@ivosung tmp-447ad33f147bb98$ pip list
cffi (0.8.1)
CherryPy (3.2.5)
cryptography (0.2.1)
Jinja2 (2.7.2)
MarkupSafe (0.18)
pip (1.6.dev1)
pipa (0.1.0)
pycparser (2.10)
pyOpenSSL (0.14)
setuptools (2.1)
six (1.5.2)
(tmp-447ad33f147bb98)ivo@ivosung tmp-447ad33f147bb98$ pip freeze
cffi==0.8.1
CherryPy==3.2.5
cryptography==0.2.1
Jinja2==2.7.2
MarkupSafe==0.18
pipa==0.1.0
pycparser==2.10
pyOpenSSL==0.14
six==1.5.2
(tmp-447ad33f147bb98)ivo@ivosung tmp-447ad33f147bb98$ pip freeze -r r.txt
pipa==0.1.0
pyOpenSSL==0.14
MarkupSafe==0.18
CherryPy==3.2.5
Jinja2==2.7.2
## The following requirements were added by pip --freeze:
cffi==0.8.1
cryptography==0.2.1
pycparser==2.10
six==1.5.2

@Ivoz
Copy link
Contributor Author

Ivoz commented Feb 28, 2014

After above commit:

(tmp-447ad33f147bb98)ivo@ivosung tmp-447ad33f147bb98$ pip freeze
-e git+https://github.com/Ivoz/calldules.git@93a0e74b13e1860cbb097281f8979b52ec7ada98#egg=calldules-origin/master
cffi==0.8.1
CherryPy==3.2.5
cryptography==0.2.1
Jinja2==2.7.2
MarkupSafe==0.18
pipa==0.1.0
pycparser==2.10
pyOpenSSL==0.14
six==1.5.2
(tmp-447ad33f147bb98)ivo@ivosung tmp-447ad33f147bb98$ pip freeze -r r.txt 
pipa==0.1.0
pyOpenSSL==0.14
MarkupSafe==0.18
-e git+https://github.com/Ivoz/calldules.git@93a0e74b13e1860cbb097281f8979b52ec7ada98#egg=calldules-master
CherryPy==3.2.5
Jinja2==2.7.2
## The following requirements were added by pip --freeze:
cffi==0.8.1
cryptography==0.2.1
pycparser==2.10
six==1.5.2
(tmp-447ad33f147bb98)ivo@ivosung tmp-447ad33f147bb98$ cat r.txt 
pipa==0.1.0
pyOpenSSL==0.13.1
MarkupSafe==0.18
-e git+https://github.com/Ivoz/calldules.git#egg=calldules
CherryPy==3.2.4
Jinja2==2.7.2

@qwcode
Copy link
Contributor

qwcode commented Mar 1, 2014

not sure about putting editables at the top
pip list doesn't do that.
if you can fix the tests w/o doing that, I think I'd prefer that, unless someone else really likes the editables that way

@Ivoz
Copy link
Contributor Author

Ivoz commented Mar 2, 2014

#1602 (comment)

I also like them better at the top, makes them obvious and easier to read imho as you know where to expect the big long vcs url should be in the output.

@qwcode
Copy link
Contributor

qwcode commented Mar 2, 2014

just thinking it should be consistent with pip list
@pfmoore , @dstufft care to vote on the ordering of pip list and pip freeze
the question at hand being whether to list editables first.
at first, this PR was just about case insensitive ordering, but this came up as well.

@piotr-dobrogost
Copy link

When there's name given as in #egg=calldules it would be best to make use of it when sorting. In cases where it's not present (it's not required AFAIK) not mixing editables with the rest sounds like something most people would expect.

@pfmoore
Copy link
Member

pfmoore commented Mar 2, 2014

I don't really care what the order is (I don't use pip freeze much myself). But if we do agree a particular order, can we make sure it's documented so that we don't have someone later coming along and requesting a different order? That means I don't consider this patch complete without a corresponding docs patch.

Also note that sorting is non-trivial in a Unicode world - I'm pretty sure package names are restricted to something like ASCII, so it should be a non-issue here but I don't know where that's documented.

@Ivoz
Copy link
Contributor Author

Ivoz commented Mar 2, 2014

@piotr-dobrogost the #egg=<package> fragment is almost always required at the moment, especially for vcs source checkouts. I'd also suspect that's the most common usage of editable installs which need to be listed/frozen. So the latter case you're talking about, IMO, would be almost non-existent. I still think it looks nicer to sort the vcs editables at the top, instead of scattered amongst other packages.

@pfmoore sure, I'll see if I can find a good place to document the behaviour somewhere, as long as peeps would like this in in some form or another.

@dstufft
Copy link
Member

dstufft commented Mar 2, 2014

Yes package names are mostly alpha numerical. The full rules are here: http://python.org/dev/peps/pep-0426/#name

@qwcode
Copy link
Contributor

qwcode commented Mar 3, 2014

to be clear, my vote is for:

  1. case-insensitive ordering for list and freeze (this only changes freeze, which is currently case sensitive)
  2. and to not specifically put editables at the top (neither list or freeze currently does this)

the reason for #2, is I think pip list looks ok as it is, and I'd like freeze to be consistent with list, even if it is somewhat visually chaotic.

@Ivoz
Copy link
Contributor Author

Ivoz commented Mar 8, 2014

Added docs, reverted putting editables at top

@qwcode
Copy link
Contributor

qwcode commented Mar 8, 2014

ok, I'll merge once travis build completes.

qwcode added a commit that referenced this pull request Mar 8, 2014
Freeze package listing in case insensitive order
@qwcode qwcode merged commit c4d3e7e into pypa:develop Mar 8, 2014
@Ivoz Ivoz deleted the lowercase branch March 8, 2014 19:10
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants