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 Union with Literals #69

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

rhaps0dy
Copy link
Contributor

@rhaps0dy rhaps0dy commented May 10, 2024

NOTE: currently this PR has a test that fails: serializing a Keyed union with literals. Fixing this requires changing the signature of get_type_id for UnionMembers for a bunch of classes, which I don't feel like doing now. Do I need to do it?

Partially Fixes #47

Unions with Literal types on them could not be serialized or deserialized. They now can.

@rhaps0dy
Copy link
Contributor Author

Thoughts on this PR? IS this something you'd like to address?

@NiklasRosenstein
Copy link
Owner

Hey @rhaps0dy, while I'm failing to see much practical applications 😉 this is a legit type of type hint that seems like it should deserializable by Databind. I think we could just say that the nested union serialization format is not supported when a Literal is involved in the union members and error out.

@rhaps0dy
Copy link
Contributor Author

Oh, I can give you practical applications for this one!

Here's my configuration for describing a convolution layer, pay attention to the padding type:

@dataclasses.dataclass(frozen=True)
class ConvConfig:
    features: int
    kernel_size: Tuple[int, ...]
    strides: Tuple[int, ...]
    padding: Literal["SAME", "VALID"] | Sequence[Tuple[int, ...]] = "SAME"
    use_bias: bool = True
    initialization: Literal["torch", "lecun"] = "lecun"

from e.g. the Torch docs (https://pytorch.org/docs/stable/generated/torch.nn.Conv2d.html):

padding controls the amount of padding applied to the input. It can be either a string {‘valid’, ‘same’} or an int / a tuple of ints giving the amount of implicit padding applied on both sides.

this would imply a type of Literal["same", "valid"] | Sequence[int | Tuple[int, ...]], which I haven't used because this is for Jax code, but you see the use.

--

The actual reason I fixed it is that I thought there were very few [bug] issues in the tracker and maybe I could make PRs for all of them. But now I'm making use of it.

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.

no deserializer for Union[Literal[...], str] and payload of type str
2 participants