Skip to content

Conversation

@dariomesic
Copy link
Contributor

Purpose

This PR addresses the TODO in AbstractRaises._check_match which notes that the error message for a regex mismatch can be confusing.

The previous message used Regex: and Input:, which could be ambiguous.

This change updates the _fail_reason string to be more explicit:

  • Regex: -> Expected regex:
  • Input: -> Actual message:

This makes the failure message clearer and easier to debug for users, as requested in the code comment.

@dariomesic dariomesic marked this pull request as ready for review October 28, 2025 12:49
Copy link
Member

@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.

You might want to remove the comment at the same time ?

Co-authored-by: Pierre Sassoulas <[email protected]>
@dariomesic
Copy link
Contributor Author

You might want to remove the comment at the same time ?

Yeah ofcourse, I didn't want to remove it myself until they are okay with it

dariomesic and others added 2 commits October 28, 2025 14:01
Comment saying 
"""
# I don't love "Regex"+"Input" vs something like "expected regex"+"exception message"
# when they're similar it's not always obvious which is which"
"""
is now removed for clarity
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

I left a suggestion. Also need to fix the failing tests.

@Pierre-Sassoulas Pierre-Sassoulas added the skip news used on prs to opt out of the changelog requirement label Oct 29, 2025
@dariomesic dariomesic force-pushed the clarify-raises-match-error branch 4 times, most recently from c480ceb to bf27097 Compare October 29, 2025 13:28
@dariomesic dariomesic force-pushed the clarify-raises-match-error branch from 3359dd2 to 582107b Compare October 29, 2025 13:45
@dariomesic dariomesic closed this Oct 29, 2025
@dariomesic
Copy link
Contributor Author

Will create another one with clear instructions and commits

@dariomesic dariomesic deleted the clarify-raises-match-error branch October 29, 2025 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news used on prs to opt out of the changelog requirement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants