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

fix(patterns): fix composing AnyOf and AllOf patterns #7

Merged
merged 7 commits into from
Aug 6, 2024

Conversation

cpcloud
Copy link
Collaborator

@cpcloud cpcloud commented Aug 6, 2024

  • test: add test for and-ing and or-ing allof/anyof
  • fix(patterns): reference the correct attribute in sugar

@cpcloud cpcloud force-pushed the fix-inners-reference branch 2 times, most recently from 50f57f0 to 1f32dfa Compare August 6, 2024 16:37
@cpcloud
Copy link
Collaborator Author

cpcloud commented Aug 6, 2024

Seems like inners can't be accessed from Cython, the tests pass with pure Python.

@kszucs
Copy link
Owner

kszucs commented Aug 6, 2024

Seems like inners can't be accessed from Cython, the tests pass with pure Python.

Right, you need to cast the object to the extension type first:

return AnyOf(*self.inners, *other.inners)

to

left: AnyOf = cython.cast(AnyOf, self)
right: AnyOf = cython.cast(AnyOf, other)
return AnyOf(*left.inners, *right.inners)

@cpcloud
Copy link
Collaborator Author

cpcloud commented Aug 6, 2024

Ah, roger that. Fixing it now.

elif isinstance(self, AllOf):
return AllOf(*self.patterns, other)
elif isinstance(other, AllOf):
return AllOf(self, *other.patterns)
Copy link
Collaborator Author

@cpcloud cpcloud Aug 6, 2024

Choose a reason for hiding this comment

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

I dunno, maybe the original but with casts is better, the entire implementation is in a single place 🤔

@cpcloud cpcloud force-pushed the fix-inners-reference branch from d017ee3 to f6a2074 Compare August 6, 2024 17:09
@kszucs kszucs changed the title fix inners reference fix(patterns): fix composing AnyOf and AllOf patterns Aug 6, 2024
Copy link
Owner

@kszucs kszucs left a comment

Choose a reason for hiding this comment

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

Sweet, thanks Phillip!

@kszucs kszucs merged commit 49549bf into kszucs:main Aug 6, 2024
14 checks passed
@cpcloud cpcloud deleted the fix-inners-reference branch August 6, 2024 19:49
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