Skip to content

Support ClassVar on Struct types - #281

Merged
jcrist merged 1 commit into
mainfrom
struct-classvar
Jan 28, 2023
Merged

Support ClassVar on Struct types#281
jcrist merged 1 commit into
mainfrom
struct-classvar

Conversation

@jcrist

@jcrist jcrist commented Jan 28, 2023

Copy link
Copy Markdown
Member

Previously this was unsupported.

Postponed annotation evaluations are a bit of a tricky issue here. We currently don't parse type annotations at import time (we do so lazily on the first decode call). This is nice for two reasons:

  • It keeps import times quick. Especially for CLI applications where not all struct types will be decoded in a single invocation, it pays off to avoid parsing types unless needed.
  • It plays better with recursive structures, since we won't ever parse a type that isn't fully defined.

However, we do need to know which attributes are/aren't fields at Struct type definition time, which means we'll have to detect if an attribute is a ClassVar without eval-ing the annotation string. This is a bit tricky to get right (enough), without compromising on performance.

Our solution is to only accept ClassVar annotations of the following forms:

  • ClassVar and ClassVar[<type>]
  • typing.ClassVar and typing.ClassVar[<type>]

Importing either ClassVar or typing under an alias won't be detected with postponed annotations. I did a quick survey of projects and couldn't find a use of ClassVar that didn't fall into one of these cases, so I think we're fine here.

Note that this doesn't rely solely on string comparisons, that's just a fast-filter to discard the 99% of fields that aren't class variables. So the following annotation is properly detected to not be a class variable:

from msgspec import Struct

ClassVar = list

class Example(Struct):
    x: ClassVar[int]  # msgspec can tell that this isn't a ClassVar

With this optimization, ClassVar detection has a negligible effect on struct import performance, while still supporting all the common cases.

Fixes #276.

Previously this was unsupported.

Postponed annotation evaluations are a bit of a tricky issue here. We
currently don't parse type annotations at import time (we do so lazily
on the first decode call). This is nice for two reasons:

- It keeps import times quick. Especially for CLI applications where not
  all struct types will be decoded in a single invocation, it pays off
  to avoid parsing types unless needed.
- It plays better with recursive structures, since we won't ever parse a
  type that isn't fully defined.

However, we do need to know which attributes are/aren't fields at Struct
type definition time, which means we'll have to detect if an attribute
is a `ClassVar` *without* eval-ing the annotation string. This is a bit
tricky to get right (enough), without compromising on performance.

Our solution is to only accept `ClassVar` annotations of the following
forms:

- `ClassVar` and `ClassVar[<type>]`
- `typing.ClassVar` and `typing.ClassVar[<type>]`

Importing either `ClassVar` or `typing` under an alias won't be detected
with postponed annotations. I did a quick survey of projects and
couldn't find a use of `ClassVar` that didn't fall into one of these
cases, so I think we're fine here.

Note that this doesn't rely solely on string comparisons, that's just a
fast-filter to discard the 99% of fields that aren't class variables. So
the following annotation is properly detected to not be a class
variable:

```python
from msgspec import Struct

ClassVar = list

class Example(Struct):
    x: ClassVar[int]  # msgspec can tell that this isn't a ClassVar
```

With this optimization, `ClassVar` detection has a negligible effect on
struct import performance, while still supporting all the common cases.
@jcrist jcrist mentioned this pull request Jan 28, 2023
@jcrist
jcrist merged commit 4d47eec into main Jan 28, 2023
@jcrist
jcrist deleted the struct-classvar branch January 28, 2023 21:56
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.

Issue with ClassVar

1 participant