Skip to content

Conversation

alukach
Copy link
Member

@alukach alukach commented Mar 14, 2025

This PR exposes the Expr.matches functionality to Python.

When writing tests, I was arguably a bit heavy handed by way of:

  1. Reworking imports slightly as I found it odd that we were importing the entirety of the cql2 module and then some specific classes despite already having access to them from the module
  2. Adding a parameterized test, which doesn't seem to be used in this project

Feel free to push back on style or scope of changes, I was very much stumbling my way through this.

@alukach alukach force-pushed the py/matches branch 2 times, most recently from a720674 to 652f464 Compare March 14, 2025 20:10
@alukach alukach requested review from bitner, gadomski and kylebarron and removed request for bitner and gadomski March 14, 2025 20:10
Copy link
Collaborator

@gadomski gadomski left a comment

Choose a reason for hiding this comment

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

Reworking imports slightly as I found it odd that we were importing the entirety of the cql2 module and then some specific classes despite already having access to them from the module

This is a me thing, I like to import classes directly but use functions w/ a namespace — habit from somewhere, don't remember where. Totally fine w/ your rework.

@bitner bitner merged commit a39b3f2 into main Mar 17, 2025
1 check passed
@bitner bitner deleted the py/matches branch March 17, 2025 20:41
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.

3 participants