Include editable locations in JSON output of list #7670
Conversation
| info['location'] = dist.location | ||
| if options.verbose >= 1: | ||
| info['installer'] = get_installer(dist) |
There was a problem hiding this comment.
It does not feel right to me to have the verbose flag influence the content of a json output.
What is also not completely right is to have the location field having two semantics: the editable project location and the installed location. These are two different things that should go in different fields IMO.
There was a problem hiding this comment.
The location semantic would also affect the column format. It has also existed for a while now so the compatibility issue would be important.
Regarding verbosity, I have no problem always outputting all fields.
There was a problem hiding this comment.
I think the concern was backwards compatibility?
If this mode of output does not feel very reasonable, I don't mind changing it but I'm also not sure if there's a clear transition path to introduce these additional fields and who it might affect if we do so.
There was a problem hiding this comment.
Yeah, agreed. One solution I can think of is to introduce a --fields option that explicitly tells pip what fields to output (the default being to infer from verbosity).
There was a problem hiding this comment.
I think our tests breaking because of new fields does not necessarily indicate that there are people out there relying on the absence of such fields. So I'd still be inclined to add the fields unconditionally and adapting our tests.
uranusjr
force-pushed
the
list-location-editable
branch
2 times, most recently
from
b40c7f0
to
607dbea
Compare
|
So I tried including the fields unconditionally, and it broke about 100 cases in the test suite because they don’t expect IMO this is a sign people are probably relying on this behaviour. Let’s stick to mimicking the |
uranusjr
force-pushed
the
list-location-editable
branch
from
b35f90b
to
317b187
Compare
|
@uranusjr could you rebase this PR? Or close this and open a new one from the same branch, so that the Github checks get updated? |
uranusjr
force-pushed
the
list-location-editable
branch
from
33f62e2
to
80749dd
Compare
This matches the JSON format with the default columns format.
uranusjr
force-pushed
the
list-location-editable
branch
from
80749dd
to
b9dbeb9
Compare
|
Rebased, waiting for CI. |
github-actions
bot
added
the
needs rebase or merge
|
@uranusjr Do you reckon you'd have time in the next 6 months or so, to update this PR? If not, let's close this out for now so that someone else could come in and pick this up? |
|
I think this is no longer relevant now we’re implementing PEP 660 and changing the location outputs. We can make that field always visible in the JSON output instead without must (any?) backward compatibility concerns. (Please comment @sbidoul if you feel otherwise.) |
|
Yup, this is all covered in #10249. |
Fix #7664. This matches the JSON format with the default columns format.
Probably need to come up with a test to make sure the two formats match.