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

Registered functions with pydantic class arguments can't be parsed #59

Open
timothyjlaurent opened this issue Dec 2, 2020 · 0 comments
Labels
enhancement New feature or request

Comments

@timothyjlaurent
Copy link

timothyjlaurent commented Dec 2, 2020

Consider this:

from pydantic import BaseModel

@registry.dim('v1')
class Dim(BaseModel):
   length: int

@registry.architecture('v1')
def load_architecture(foo: str, dim: Dim):
    ...

config str = """
[model]
@architecture = 'v1'
length = 6
[model.dim]
@dim = 'v1'
length = 6
"""

So the problem with this is that thinc.Config will generate an ArgumentModel from the registered architecture function

https://github.com/explosion/thinc/blob/adeb99ab7f005228c5a45a8c755929a6bb081d78/thinc/config.py#L910

in config._fill

        validation.update(result.dict(exclude=exclude_validation))
        filled, final = cls._update_from_parsed(validation, filled, final) # <- this breaks

in the first line both validation and results both have the dim attribute at the desired pydantic class.

    dim: <Dim length=6>

however on the call to results.dict() the pydantic model is transformed to a dictionary as is the behavior of Pydantic .dict method. Then in _update_from_parsed()
https://github.com/explosion/thinc/blob/adeb99ab7f005228c5a45a8c755929a6bb081d78/thinc/config.py#L932

            if isinstance(value, dict):
                filled[key], final[key] = cls._update_from_parsed(
                    value, filled[key], final[key]
                )

It iterates over validation items, thinks that dim is a dict and tries to set the key on the pydantic model in final[key].

I think there needs to be some way for Config to tell apart configs that come with a pydantic Schema and those that come from functions that happen to contain pydantic models. (Luckily I also have attrs in this project so could avoid erroneously casting it to a dict after figuring out the problem)

@adrianeboyd adrianeboyd transferred this issue from explosion/thinc Dec 20, 2023
@svlandeg svlandeg added the enhancement New feature or request label Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants