Pyreverse: print package import stats #8974
Conversation
Pierre-Sassoulas
added
the
Needs review 🔍
There was a problem hiding this comment.
I also think there can be value in printing the module names.
But it not necessarily needs to be the "default", and having the stats is also quite nice.
My suggestion would be:
- keep the stats and output them by default
- add a verbose option
- output the module names only in verbose mode
|
Sounds good! I'll update this PR with a force-push. |
nickdrozd
force-pushed
the
pyreverse-printing
branch
from
f478a9b to
4255e32
Compare
|
Updated the PR to add a |
for more information, see https://pre-commit.ci
pylint/pyreverse/writer.py
Outdated
| print(f"Modules: {len(module_info)}") | ||
| print(f'Imports: {sum(mod["imports"] for mod in module_info.values())}') |
There was a problem hiding this comment.
Not sure if it should be in non verbose mode but I'll let Andreas decide.
| print(f"Modules: {len(module_info)}") | |
| print(f'Imports: {sum(mod["imports"] for mod in module_info.values())}') | |
| if verbose: # TODO add verbose in this function parameters if we do that | |
| print(f"Modules: {len(module_info)}") | |
| print(f'Imports: {sum(mod["imports"] for mod in module_info.values())}') |
There was a problem hiding this comment.
My thought was to print that by default. After all, it isn't debugging information, it's just information. There are questions about module relations that Pyreverse is well-suited to answer since it's looking at module relations anyway. So I figure, why not print it?
There are lots of other stats that could be added. Which module is the most / least imported? Which module has the most / fewest imports? How many transitive dependencies are there? I've found that some design and organization improvements can reveal themselves through investigating anomalous import stats.
There was a problem hiding this comment.
I guess clever warning about module stats should go in pylint or another standalone tool (there's a bunch of clever high level design check that are only in the litterature on arxiv or design books) . I see pyreverse more as a graphical display of uml diagram
There was a problem hiding this comment.
As for the output I would print it by default, I feel it gives a nice concise feedback how much was analysed.
What do you think of printing it like "Analysed X modules with a total of Y imports"?
Alternatively at least:
Analysed modules: X
Total imports: Y
As for the other stats, I agree with Pierre. This can be very useful information, but I don't think it belongs in pyreverse. If at all in Pylint and not a dedicated design. checker, it would be more of a use case for a report.
There was a problem hiding this comment.
I like "Analysed X modules with a total of Y imports" in one line 👍
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8974 +/- ##
=======================================
Coverage 95.75% 95.76%
=======================================
Files 173 173
Lines 18604 18616 +12
=======================================
+ Hits 17815 17827 +12
Misses 789 789
|
Pierre-Sassoulas
added
Enhancement ✨
|
To test the verbose option and bump the coverage you may take a look at how other tests checking the output of running |
Type of Changes
Description
Here is what Pyreverse currently prints:
Here is what it prints with this change:
There are many ways this could be changed, but I think this is a good first attempt for more useful printing.
Closes #8973