Skip to content

Address no files to check pre-commit message#1077

Open
DimitriPapadopoulos wants to merge 5 commits into
pydicom:mainfrom
DimitriPapadopoulos:pre-commit_src
Open

Address no files to check pre-commit message#1077
DimitriPapadopoulos wants to merge 5 commits into
pydicom:mainfrom
DimitriPapadopoulos:pre-commit_src

Conversation

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Mar 7, 2026

Reference issue

#1075 (comment)

It appears MyPy hasn't been run for ages:

Found 4081 errors in 80 files (checked 113 source files)

I think it doesn't make sense to try to enable it in CI until there errors are addressed.

Tasks

  • Unit tests added that reproduce issue or prove feature is working
  • Fix or feature added
  • Documentation and examples updated (if relevant)
  • Unit tests passing and coverage at 100% after adding fix/feature
  • Type annotations updated and passing with mypy
  • Apps updated and tested (if relevant)

check for broken symlinks............................(no files to check)Skipped
ruff check...........................................(no files to check)Skipped
mypy.................................................(no files to check)Skipped
Found 4081 errors in 80 files (checked 113 source files)
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.87%. Comparing base (a9b9312) to head (c17fbf1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1077   +/-   ##
=======================================
  Coverage   99.87%   99.87%           
=======================================
  Files          28       28           
  Lines        8984     8986    +2     
=======================================
+ Hits         8973     8975    +2     
  Misses         11       11           
Files with missing lines Coverage Δ
pynetdicom/_handlers.py 100.00% <100.00%> (ø)
pynetdicom/ae.py 100.00% <100.00%> (ø)
pynetdicom/association.py 100.00% <100.00%> (ø)
pynetdicom/dimse_messages.py 100.00% <ø> (ø)
pynetdicom/dsutils.py 100.00% <100.00%> (ø)
pynetdicom/events.py 100.00% <100.00%> (ø)
pynetdicom/pdu_items.py 100.00% <100.00%> (ø)
pynetdicom/presentation.py 100.00% <100.00%> (ø)
pynetdicom/service_class.py 100.00% <100.00%> (ø)
pynetdicom/utils.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@scaramallion
Copy link
Copy Markdown
Member

scaramallion commented Mar 7, 2026

Found 4081 errors in 80 files (checked 113 source files)

It shouldn't be that bad, I at least ran it locally when doing the v3 release.

80 files? Is it checking the tests directory as well?

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor Author

DimitriPapadopoulos commented Mar 8, 2026

I haven't changed what it checks, only test_files are excluded.

Actually, this is a known pre-commit feature: in order to parallelize checks, it runs hooks on each file individually and explicitly. Unfortunately, explicitly passing files to the underlying software may override its exclude option. That's one reason why ruff has force-exclude option:

This is useful for pre-commit, which explicitly passes all changed files to the ruff-pre-commit plugin, regardless of whether they're marked as excluded by Ruff's own settings.

Neither codespell nor MyPy have such an option. Therefore, files have to be excluded twice, once in the MyPy settings in case it is run standalone, and once in pre-commit settings. That's rather unfortunate, but there's no way around that.

@DimitriPapadopoulos
Copy link
Copy Markdown
Contributor Author

Much better after getting pre-commit to exclude tests and the like:

Found 41 errors in 10 files (checked 28 source files)

This time, make sure to exclude files also when run with pre-commit.
@DimitriPapadopoulos DimitriPapadopoulos force-pushed the pre-commit_src branch 4 times, most recently from bfad6cd to 678719c Compare March 8, 2026 10:18
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.

2 participants