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

Have duplicate metadata and invalid req strings honor --warn option #357

Merged
merged 7 commits into from
Apr 28, 2024

Conversation

kemzeb
Copy link
Collaborator

@kemzeb kemzeb commented Apr 20, 2024

This resolves #355 by making changes and refactors to the warning logic. It does so by introducing a module-level singleton "WarningPrinter" object and refactors the code in such a way to integrate this object for it to be used.

@kemzeb
Copy link
Collaborator Author

kemzeb commented Apr 25, 2024

Still working on this, I'll see if I can finish this and open it for review by the end of today.

More specifically, the duplicate metadata and invalid req string
warnings.
@kemzeb
Copy link
Collaborator Author

kemzeb commented Apr 26, 2024

Did a redesign and this seems to be a better implementation than the last. Just need to add coverage tests then I'll open this PR for review

@kemzeb
Copy link
Collaborator Author

kemzeb commented Apr 26, 2024

I think this is ready to go, going to open this for review

@kemzeb kemzeb marked this pull request as ready for review April 26, 2024 16:31
@kemzeb
Copy link
Collaborator Author

kemzeb commented Apr 26, 2024

Here are my design decisions:

  • Decided to keep the warning printing logic in their respective modules since they appear to be tightly coupled together
  • Included a _warning module; did try to add it to render but I encountered dependency cycle errors
  • Implemented a WarningPrinter class, where I also exposed a shared module-level singleton of it
  • Decided that the duplicate metadata warning should not be considered a "fail" warning when --warn fail is passed. My reasoning for this is that Python allows these duplicate distributions to exist and pip actively ignores them.
  • I thought it would be nice for WarningPrinter.print_multi_line() to pass what file to write to to the callback so that we avoid throwing sys.stderr everywhere, but due to an odd long-standing bug with the capsys/capfd fixtures, it won't properly capture sys.stderr if we were to pass it down to the callback.

@kemzeb kemzeb changed the title Have new warnings honor --warn option and refactor warning logic Have duplicate metadata and invalid req strings honor --warn option Apr 26, 2024
src/pipdeptree/_models/dag.py Show resolved Hide resolved
src/pipdeptree/_discovery.py Show resolved Hide resolved
src/pipdeptree/_warning.py Outdated Show resolved Hide resolved
@xiacunshun
Copy link
Collaborator

xiacunshun commented Apr 28, 2024

Ignore my previous deleted comment, I've reconsidered and the handling of "duplicated" here should be fine.

@xiacunshun
Copy link
Collaborator

No questions for me here. But I don't have the approve auth, so maybe need to wait for @gaborbernat to take a further look.

@kemzeb
Copy link
Collaborator Author

kemzeb commented Apr 28, 2024

Great, will release 2.19.0 after merging this

@kemzeb kemzeb merged commit 4a5f90a into tox-dev:main Apr 28, 2024
10 checks passed
@kemzeb kemzeb deleted the refactor-warning-logic branch April 28, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider --warn option when dealing with duplicate metadata and invalid requirement string warnings
3 participants