Skip to content

Conversation

@alexfikl
Copy link
Collaborator

I was updating inducer/boxtree#56 and noticed that it also needs to handle string annotations, but with a slightly different is_array_field predicate.

This refactors the get_annotations logic into a separate function so that it can be reused.

Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks! Just a few minor nits/questions, but generally LGTM!

Comment on lines 109 to 112
if __debug__:
# NOTE: this should never happen due to using `inspect.get_annotations`
if isinstance(field_type, str):
raise TypeError(f"String annotation on field '{f.name}' not supported.")
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
if __debug__:
# NOTE: this should never happen due to using `inspect.get_annotations`
if isinstance(field_type, str):
raise TypeError(f"String annotation on field '{f.name}' not supported.")
assert not isinstance(field_type, str)

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@alexfikl alexfikl force-pushed the refactor-dataclass-types branch from c44687e to f705a8a Compare December 17, 2024 17:21
@inducer inducer enabled auto-merge (rebase) December 17, 2024 17:31
@inducer inducer merged commit 9ac8bc8 into inducer:main Dec 17, 2024
12 checks passed
@alexfikl alexfikl deleted the refactor-dataclass-types branch December 17, 2024 17:34
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