Skip to content

Conversation

@cbowie-mp
Copy link
Contributor

Instructions

  1. PR target branch should be against development
  2. PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
  3. PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

  • {provide a thorough description of the changes}

Testing Plan

  • Was this tested locally? If not, explain why.
  • {explain how this has been tested, and what, if any, additional testing should be done}

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

/**
* @title BooleanExpression
*/
export type BooleanExpression =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed expressions to be based on the type they resolve to rather than the type of the operand. This makes it possible for us to restrict the types of operands for various operators.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The transpiler we're using to go from TS to JSON schema looks for @title in a comments so we can get rid of the add titles to schema script.

I added the necessary comments to all the types we're exposing and double checked the python doesn't have types with incorrect names.


class PathExpression(
RootModel[Union[bool, float, str, UnaryPathExpression, BinaryPathExpression]]
class Expression(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One annoyance here that might be resolvable, the code gen tool to convert JSON schema to python puts this expression out of order. It puts it above NumberExpression so the python is invalid... I think that's just a bug with the tool, but it's easy enough to manually move it.

@cbowie-mp cbowie-mp changed the title Refactor/re organize hierarchy refactor: Re-organize class hierarchy Jul 1, 2025
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