Skip to content

Add Pyreverse primer#10974

Open
Julfried wants to merge 12 commits intopylint-dev:mainfrom
Julfried:pyreverse-primer
Open

Add Pyreverse primer#10974
Julfried wants to merge 12 commits intopylint-dev:mainfrom
Julfried:pyreverse-primer

Conversation

@Julfried
Copy link
Copy Markdown
Contributor

Type of Changes

Type
🐛 Bug fix
✨ New feature
🔨 Refactoring
📜 Docs

Description

Based on the discussion in #10820 I tried to implement a pyreverse primer. Currently contains a lot of duplicate code which can likely be unified with the pylint primer. This is an initial attempt, feedback is welcome :)

@Pierre-Sassoulas Pierre-Sassoulas added pyreverse Related to pyreverse component Needs review 🔍 Needs to be reviewed by one or multiple more persons primer labels Apr 18, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

❌ Patch coverage is 90.71038% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.13%. Comparing base (34b7ec5) to head (fd77cee).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../testutils/_primer/pyreverse_primer_run_command.py 63.63% 12 Missing ⚠️
...ylint/testutils/_primer/pyreverse_primer_target.py 87.50% 2 Missing ⚠️
pylint/testutils/_primer/pyreverse_primer.py 98.14% 1 Missing ⚠️
...lint/testutils/_primer/pyreverse_primer_command.py 95.00% 1 Missing ⚠️
...tutils/_primer/pyreverse_primer_compare_command.py 98.33% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (90.71%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #10974      +/-   ##
==========================================
- Coverage   96.18%   96.13%   -0.06%     
==========================================
  Files         178      183       +5     
  Lines       19662    19845     +183     
==========================================
+ Hits        18911    19077     +166     
- Misses        751      768      +17     
Files with missing lines Coverage Δ
pylint/testutils/_primer/pyreverse_primer.py 98.14% <98.14%> (ø)
...lint/testutils/_primer/pyreverse_primer_command.py 95.00% <95.00%> (ø)
...tutils/_primer/pyreverse_primer_compare_command.py 98.33% <98.33%> (ø)
...ylint/testutils/_primer/pyreverse_primer_target.py 87.50% <87.50%> (ø)
.../testutils/_primer/pyreverse_primer_run_command.py 63.63% <63.63%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Julfried
Copy link
Copy Markdown
Contributor Author

@Pierre-Sassoulas I noticed that you have been working very actively on the pylint primer so it might be better to convert this to a draft for now

@github-actions
Copy link
Copy Markdown
Contributor

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

This comment was generated for commit fd77cee

@Pierre-Sassoulas
Copy link
Copy Markdown
Member

I'm going to add the 'changed' messages concept for the pylint side, but it should be compatible. Maybe we should find what's generic before I commit to what I was planning ? (Tbh the Claude implementation is very verbose and not satisfactory right now)

@Julfried
Copy link
Copy Markdown
Contributor Author

I assume you are referring to #10973? I think it makes sense to make the primer output more structured and I like the new format better. Looking at it, both implementations look quite similar ==> so I would say the following things can likely be unified in a base Primer base class:

  1. Argparse setup with subparsers (besides some arguments the subcommands are almost identical)
  2. Package loading from config with Python version filtering

Main differences are command implementations and the way targets are defined (pylint primer will reference a full package) while the pyreverse primer operates on classes as targets. Also comparator.py cannot be reused because pyerverse is just a simple text based diff.

One thing that seems wasteful is to run pyreverse primer in a separate workflow ==> should run in the same workflow with pyreverse as an optional step (based on pyreverse label or some other trigger).

@Pierre-Sassoulas
Copy link
Copy Markdown
Member

I was thinking of #10914 (where there's a diff introduced for "changed message", but I'm not sure if it's that easy to reuse)

Maybe we can add the arg for the package we want to use pyreverse on in the json defining the repo we prime ?

@Julfried
Copy link
Copy Markdown
Contributor Author

I see, I do not think there is much logic to be shared with the Pyreverse primer because it operates on sets of messages instead of bare line diffs. I agree with you ==> prior to this PR we should first focus on a common config interface that can be used for both primers. For rest the logic is quite different and needs individual implementations anyway

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs review 🔍 Needs to be reviewed by one or multiple more persons primer pyreverse Related to pyreverse component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants