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

Add support for JSON output format #2085

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

chrysle
Copy link
Contributor

@chrysle chrysle commented May 3, 2024

Very WIP right now, I'll add (and fix) tests once everyone is satisfied with the implementation. Thus, I'd appreciate some feedback and suggestions.

Closes #1377.

Contributor checklist
  • Included tests for the changes.
  • PR title is short, clear, and ready to be included in the user-facing changelog.
Maintainer checklist
  • Verified one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@chrysle chrysle added logging Related to log or console output feature Request for a new feature cli Related to command line interface things writer Related to results output writer component labels May 3, 2024
@AndydeCleyre
Copy link
Contributor

Thanks!

I think the output is looking great.

Some things that may surprise in the current state:

  • the JSON types of each entry's hashable and markers are string, but might conceivably be boolean and list respectively
  • in addition to the JSON output, the text file is written (as usual)
  • line values may technically be multiple lines depending on annotation style

They don't necessarily need "fixing." But it may be more common to want to generate the JSON without also writing the txt 🤷🏼 .

Oh I think via is currently a string and only includes one package when the annotations indicate more than one.

@chrysle
Copy link
Contributor Author

chrysle commented May 4, 2024

Thanks for the quick response!

  • the JSON types of each entry's hashable and markers are string, but might conceivably be boolean and list respectively

Good point, switched to using booleans for several keys. About the list format for markers, I'm unsure how to construct it from the given Marker class. They are explicitly formatted as strings by pip per requirement via a magic method.

  • line values may technically be multiple lines depending on annotation style

Right, but as they are included for convenience so that the can directly be used to create a constraints file, I thought that wouldn't be a problem. Especially since the newline characters don't get translated to actual newlines.

But it may be more common to want to generate the JSON without also writing the txt 🤷🏼 .

I also thought so at the beginning, but then reverted the behaviour, as I think a common use case would be that a constraints file is generated for direct use and information about the constraints for further processing is collected. For other cases, it shouldn't be a big deal to add the --dry-run flag to the operation. But happy to change it if you disagree.

Oh I think via is currently a string and only includes one package when the annotations indicate more than one.

Good catch, thank you! I hacked something together (surely that can be improved) to parse the constructed output lines and get the parent requirements from them, since there is some logic in OutputWriter._format_requirement related to that that I wouldn't want to copy.

@chrysle chrysle force-pushed the pip-compile-json branch 2 times, most recently from 0847559 to 2f0b266 Compare May 10, 2024 13:33
@chrysle
Copy link
Contributor Author

chrysle commented May 10, 2024

I hacked something together (surely that can be improved)

I did that now by merging the OutputWriter._get_json function with the OutputWriter._format_requirement function (breaking a few more tests in the process). Any further thoughts?

@chrysle
Copy link
Contributor Author

chrysle commented May 11, 2024

@AndydeCleyre JSON output is now written to a requirements.json file. However, as pip-compile can only read requirements files of type text right now, the --upgrade-package mechanism is dysfunctional. Do you think that's in order?

@AndydeCleyre
Copy link
Contributor

I don't know:

  • would it make sense to still use the text output, if present, to determine upgrades? Just like usual, only with an additional json output?
  • or instead to internally render any existing json output to a temporary reqstxt and use that?

Either way could be surprising. Sorry I don't have a clear idea about this.

@chrysle chrysle added this to the 7.4.2 milestone Jun 26, 2024
@chrysle
Copy link
Contributor Author

chrysle commented Jun 27, 2024

* or instead to internally render any existing json output to a temporary reqstxt and use that?

I've adapted this suggestion, as it seemed less complex to implement; but I think we need a new Python-wide standard for a requirements.json file in the long run.

@chrysle
Copy link
Contributor Author

chrysle commented Jun 28, 2024

Cc @woodruffw, would you like to test this?

@woodruffw
Copy link

Yes, thanks for the ping! I'll try and set aside some testing time in the coming days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to command line interface things feature Request for a new feature logging Related to log or console output writer Related to results output writer component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support outputting pip-compile in a structured format (e.g. json)
3 participants