Skip to content

Check order of args in match#444

Merged
jsmariegaard merged 1 commit intomainfrom
match_order
Sep 20, 2024
Merged

Check order of args in match#444
jsmariegaard merged 1 commit intomainfrom
match_order

Conversation

@ecomodeller
Copy link
Copy Markdown
Member

@ecomodeller ecomodeller commented Sep 3, 2024

image

@ecomodeller ecomodeller marked this pull request as ready for review September 8, 2024 09:31
@jsmariegaard
Copy link
Copy Markdown
Member

@jannik-el I suggested you as reviewer for this PR which addresses the problem you encountered. Please reach out to @ecomodeller if you need guidance on how to do a PR review :-)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This change is not directly related to the issue - not aware of how specific you are on this, I don't see it as an issue if all tests passed.

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.

True. But we are pragmatic.😉

Copy link
Copy Markdown

@jannik-el jannik-el left a comment

Choose a reason for hiding this comment

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

All changes reviewed, have tested on my specific issue which caused the error and it passed, also tried some general tests to check and all looks good.
As far as I can tell all edge cases are already covered by other tests and relevant errors messages.

@jsmariegaard jsmariegaard merged commit 285ce7f into main Sep 20, 2024
@jsmariegaard jsmariegaard deleted the match_order branch September 20, 2024 10:43
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.

ms.match() AssertionError when given result and observation, and not observation and then result

3 participants