pyreverse: Add show-stdlib option

[avylove]

Type of Changes

	Type
	🐛 Bug fix
✓	✨ New feature
	🔨 Refactoring
	📜 Docs

Description

Adds an option (--show-stdlib, -L) to pyreverse. This is similar to the behavior of --show-builtin in that if this option is not included, standard library modules are not included by default.

To determine if a module is part of the standard library, sys.stdlib_module_names was used. Since this is only available in 3.10 and above, the generating code was backported to 3.7, 3.8, and 3.9. Then a list was generated. These were condensed for commonality to reduce code and create a shim.

Closes #8181

[codecov]

Codecov Report

Merging #8190 (16c2852) into main (2b3d113) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8190   +/-   ##
=======================================
  Coverage   95.71%   95.71%           
=======================================
  Files         175      175           
  Lines       18451    18455    +4     
=======================================
+ Hits        17660    17665    +5     
+ Misses        791      790    -1     

Impacted Files	Coverage Δ
pylint/pyreverse/main.py	93.87% <ø> (ø)
pylint/pyreverse/diadefslib.py	100.00% <100.00%> (+0.81%)	⬆️
pylint/testutils/pyreverse.py	98.18% <100.00%> (+0.03%)	⬆️

[Pierre-Sassoulas]

Looks pretty nice !

[DudeNr33]

Thank you for the PR!

I will do a complete review later, but already a quick remark:
The DiagramWriter class also uses logic to determine if a class or module is from the standard library (in the method get_shape_color).
It would be good to keep this consistent and use the same logic for both.

[avylove]

I will do a complete review later, but already a quick remark: The DiagramWriter class also uses logic to determine if a class or module is from the standard library (in the method get_shape_color). It would be good to keep this consistent and use the same logic for both.

I missed astroid.modutils.is_standard_module(), and probably would have just used it if I had seen it. But I feel like sys.stdlib_module_names is more accurate since it relies on a compiled list and is maintained upstream rather than the state of the current environment. Do you think it would be best to use astroid.modutils.is_standard_module() for both and then do a PR against Astroid to modify it to use sys.stdlib_module_names?

[DudeNr33]

Do you think it would be best to use astroid.modutils.is_standard_module() for both and then do a PR against Astroid to modify it to use sys.stdlib_module_names?

That is the cleanest approach in my opinion.

[DudeNr33]

The logic looks fine to me. 👍

Aside from the logic to determine which modules belong to the stdlib only some tests are missing.
You can either add a functional test (i.e. a Python module and the resulting class diagram) in tests/pyreverse/functional/class_diagrams with a suitable rcfile (example), or add a plain unittest.

For the latter you can take a look at test_inspector.test_concat_interfaces for an example how you can extract a astroid node from a code sample in a unit test.

[avylove]

Submitted pylint-dev/astroid#2015. If that gets merged, we can replace astroid.modutils.is_standard_module() everywhere.

Also added a couple tests. I went with a plain unit test because I thought it would be easier to confirm the behavior change between the options being set or not. Maybe there's an better way to implement them, not sure.

[DudeNr33]

Thank you for putting in the effort in astroid and adding tests.
This made me realize we should flip the default value to preserve the existing behavior as default.

[DudeNr33]

Suggested change

	Add new option (``--show-stdlib``, ``-L``) to pyreverse.
	Add new option (``--show-stdlib``, ``-L``) to ``pyreverse``.

Just a nitpick and I know that the existing release notes are also not consistent with this, but the majority of the docs put pylint/pyreverse in backticks.

[Pierre-Sassoulas]

We're in the alpha stage of 3.0 now, we could merge :)

[avylove]

We're in the alpha stage of 3.0 now, we could merge :)

3.0 will use Astroid >=2.15, right? So I can update this PR to use is_stdlib_module() instead of is_standard_module().

[Pierre-Sassoulas]

Definitely, I did not think of this but it would break main if you don't 😄 (You can merge upstream/main, the test will start to fail)

[avylove]

Rebased to main, fixed quotes in whatnew fragment, updated to use is_stdlib_module()

[Pierre-Sassoulas]

LGTM, but I'll let @DudeNr33 merge.

[DudeNr33]

Only one remark regarding the category of the news fragment now that we decided to change the default behavior, and then we are good to go!

[DudeNr33]

I think we should probably put this under "breaking changes" (i. e. rename the file to 8181.breaking), as it is a change in the default behavior.

[avylove]

Updated

[DudeNr33]

Thank you for this addition and the patience waiting for 3.0!
This can really help to make diagrams more readable. 👍

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 16c2852