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 the type annotation for 'argmap' #989

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

sirosen
Copy link
Collaborator

@sirosen sirosen commented Jan 28, 2025

The ArgMap type alias defined the mapping option (for
Schema.from_dict) to allow Field or type[Field], but this is
incorrect. The following construction does not work:

Schema.from_dict({"a": fields.Str})

Correct by simply removing type[Field] from the alias.

A quick included improvement is to make the handling of type narrowing
for argmap a little bit more uniform in style (we had it 3 different
ways in different places), and to use a cast to replace a type ignore
with something a little easier to read.

The ArgMap type alias defined the mapping option (for
`Schema.from_dict`) to allow `Field` or `type[Field]`, but this is
incorrect. The following construction does not work:

    Schema.from_dict({"a": fields.Str})

Correct by simply removing `type[Field]` from the alias.

A quick included improvement is to make the handling of type narrowing
for `argmap` a little bit more uniform in style (we had it 3 different
ways in different places), and to use a cast to replace a type ignore
with something a little easier to read.
@sirosen
Copy link
Collaborator Author

sirosen commented Jan 28, 2025

I need to look at our CI to figure out / decide how we want to handle marshmallow-dev being the 4.0 branch. But I'm comfortable ignoring that for today (don't have bandwidth to fix it right now).

@sirosen sirosen merged commit dc50b60 into marshmallow-code:dev Jan 28, 2025
7 of 8 checks passed
@sirosen sirosen deleted the fix-argmap-type branch January 28, 2025 21:02
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