Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update fields in LinkableElement-related classes to use tuples #1336

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

plypaul
Copy link
Contributor

@plypaul plypaul commented Jul 15, 2024

Currently, serialization of sets is not supported so this PR changes the properties field of a few LinkableElement related classes to a tuple-based implementation.

@cla-bot cla-bot bot added the cla:yes label Jul 15, 2024
@plypaul plypaul force-pushed the p--query-resolution-perf--09 branch from e35d7d1 to 24758ee Compare July 15, 2024 22:12
@plypaul plypaul force-pushed the p--query-resolution-perf--10 branch 2 times, most recently from 5e910ac to 2cba144 Compare July 16, 2024 16:54
@plypaul plypaul changed the title Use LinkableElementUnion in WhereFilterSpec Update fields in LinkableElement-related classes to use tuples Jul 16, 2024
@plypaul plypaul marked this pull request as ready for review July 16, 2024 21:06
Copy link
Contributor

@courtneyholcomb courtneyholcomb left a comment

Choose a reason for hiding this comment

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

🚢

properties: Tuple[LinkableElementProperty, ...]

def __post_init__(self) -> None: # noqa: D105
assert len(self.property_set) == len(self.properties)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add error messages to these asserts to help devs if they run into this in development?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Base automatically changed from p--query-resolution-perf--09 to main July 17, 2024 01:07
…operties` to be a tuple.

This aligns with `SerializableDataclass` since sets are current not supported.
@plypaul plypaul force-pushed the p--query-resolution-perf--10 branch from 2cba144 to 42ea1eb Compare July 17, 2024 01:15
@plypaul plypaul force-pushed the p--query-resolution-perf--10 branch from e71a515 to 9ace719 Compare July 17, 2024 22:38
@plypaul plypaul merged commit d6c2dea into main Jul 17, 2024
16 checks passed
@plypaul plypaul deleted the p--query-resolution-perf--10 branch July 17, 2024 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants