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

Error if Cython enum is used outside a typed context #5642

Merged
merged 3 commits into from Nov 9, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Aug 25, 2023

Without a cpdef to generate a corresponding Python enum, cdef enums (and enum classes) are not valid in pure Python context and should be detected accordingly.

I'm not sure if this is the best approach here and would appreciate some feedback, thanks!

Resolves #5638

@vyasr vyasr marked this pull request as ready for review August 25, 2023 20:32
@shwina
Copy link
Contributor

shwina commented Aug 26, 2023

Note; this closes #5638

@scoder
Copy link
Contributor

scoder commented Aug 27, 2023

Seems reasonable. Not sure whether it's an error or warning, but I agree that there's probably not much sense in such code. That speaks for an error.

In any case, I'll postpone this until after 3.0.2, which urgently fixes major regressions in the 3.0.1 release.

@matusvalo matusvalo added this to the 3.0.3 milestone Aug 31, 2023
@vyasr
Copy link
Contributor Author

vyasr commented Aug 31, 2023

Seems reasonable. Not sure whether it's an error or warning, but I agree that there's probably not much sense in such code. That speaks for an error.

In any case, I'll postpone this until after 3.0.2, which urgently fixes major regressions in the 3.0.1 release.

Agreed, I think error probably makes the most sense here. It's not like an unused variable or something similar harmless, it's trying to create an object in a context where it shouldn't be possible.

@scoder scoder modified the milestones: 3.0.3, 3.0.x Oct 5, 2023
@scoder scoder modified the milestones: 3.0.x, 3.0.6 Oct 30, 2023
@vyasr
Copy link
Contributor Author

vyasr commented Nov 9, 2023

@scoder do you think this is mergeable, or do you want to see a different approach?

@da-woods
Copy link
Contributor

da-woods commented Nov 9, 2023

I think this is fine but just got lost under some more serious bug fixes. I'll merge it to 3.1 now and let @scoder decide whether to backport to 3.0.6.

Thanks for the fix.

@da-woods da-woods merged commit ab8cd49 into cython:master Nov 9, 2023
63 of 64 checks passed
@da-woods da-woods modified the milestones: 3.0.6, 3.1 Nov 9, 2023
@vyasr vyasr deleted the fix/issue_5638 branch November 9, 2023 20:47
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.

[BUG] enum class initializer throws NameError
5 participants