Skip to content

Enable the warnings on the tests files#755

Merged
sbrunner merged 3 commits intomasterfrom
test-warnings
Apr 12, 2025
Merged

Enable the warnings on the tests files#755
sbrunner merged 3 commits intomasterfrom
test-warnings

Conversation

@sbrunner
Copy link
Copy Markdown
Member

Description

Check the tests files

Related Issue

#754 (comment)

Motivation and Context

Better coherence between the pre-commit and the Prospector command

@sbrunner sbrunner force-pushed the test-warnings branch 11 times, most recently from 6adc0f0 to 1cc34fb Compare April 12, 2025 15:00
@sbrunner sbrunner marked this pull request as ready for review April 12, 2025 15:02
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

@sbrunner sbrunner force-pushed the test-warnings branch 4 times, most recently from a9ba584 to c79826f Compare April 12, 2025 15:37
Copy link
Copy Markdown
Collaborator

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of work here, great job !

@sbrunner sbrunner merged commit cab53f7 into master Apr 12, 2025
7 checks passed
@carlio
Copy link
Copy Markdown
Member

carlio commented Apr 12, 2025

Awesome stuff :-)


@contextlib.contextmanager
def patch_execution(*args: str, set_cwd: Optional[Path] = None) -> Generator[None, None]:
def patch_execution(*args: str, set_cwd: Path | None = None) -> Generator[None, None]:
Copy link
Copy Markdown
Member

@carlio carlio Apr 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP 604 for union types using | wasn't added until Python 3.10 but prospector still supports 3.9 so I suggest reverting this line unless dropping 3.9 is the plan. Since 3.9 is not quite end of life yet, I'd like to avoid that.

Or is that wrong? Since CI seems to pass.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on mobile si it's hard to check but there might be an import from the future (form future import annotations)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pierre-Sassoulas You're right, there is, I missed that since it was collapsed by the GitHub diff UI :-)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then finally it's OK :-)

@sbrunner sbrunner deleted the test-warnings branch August 13, 2025 10:08
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.

4 participants