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

Mapping types are coerced to dict #2323

Closed
ofek opened this issue Feb 7, 2021 · 9 comments · Fixed by #2325
Closed

Mapping types are coerced to dict #2323

ofek opened this issue Feb 7, 2021 · 9 comments · Fixed by #2325
Labels
bug V1 Bug related to Pydantic V1.X

Comments

@ofek
Copy link
Contributor

ofek commented Feb 7, 2021

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.5.1
            pydantic compiled: True
                 install path: C:\Users\ofek\AppData\Local\Programs\Python\Python38\Lib\site-packages\pydantic
               python version: 3.8.1 (tags/v3.8.1:1b293b6, Dec 18 2019, 23:11:46) [MSC v.1916 64 bit (AMD64)]
                     platform: Windows-10-10.0.19041-SP0
     optional deps. installed: ['typing-extensions', 'email-validator']

I'm trying to use https://github.com/MagicStack/immutables

from __future__ import annotations

from typing import Mapping

from immutables import Map
from pydantic import BaseModel


class Model(BaseModel):
    test: Mapping[str, int]

print(type(Model(test=Map(key=42)).test))

produces

<class 'dict'>

Is there a way to keep the type of encountered Mappings, or create a type representing immutables.Map?

I've been at this for hours reading docs/closed issues, trying various approaches, and now I'm simply at a loss here.

@ofek ofek added the bug V1 Bug related to Pydantic V1.X label Feb 7, 2021
@rhuille
Copy link
Contributor

rhuille commented Feb 7, 2021

Hi @ofek

pydantic allows you to use arbitrary classes for validation with the setting arbitrary_types_allowed (cf. https://pydantic-docs.helpmanual.io/usage/model_config/):

from __future__ import annotations

from immutables import Map
from pydantic import BaseModel

class Model(BaseModel):
    class Config:
        arbitrary_types_allowed = True

    test: Map

print(type(Model(test=Map(key=42)).test))
# > <class 'immutables._map.Map'>

I hope this help !

@PrettyWood
Copy link
Member

PrettyWood commented Feb 7, 2021

Hi @ofek
The problem with @rhuille solution is that validation won't be done. If you still need it, see #2311 (comment)


@samuelcolvin We already support quite well Sequence by having a dedicated check and trying to return the original type
https://github.com/samuelcolvin/pydantic/blob/bd9c5723c676395363689549268738153e45a7e5/pydantic/fields.py#L649-L666
Maybe we could do something alike ? I guess something quite easy like https://github.com/PrettyWood/pydantic/pull/70/files could already handle most mapping types (works with OrderedDict, DefaultDict, Map, FrozenDict, ...)
WDYT ?

@ofek
Copy link
Contributor Author

ofek commented Feb 7, 2021

I opened a PR based on the linked comment: #2325

@ofek
Copy link
Contributor Author

ofek commented Feb 7, 2021

@PrettyWood Yours looks more robust, should I close mine?

@PrettyWood
Copy link
Member

PrettyWood commented Feb 7, 2021

You can keep yours open! Happy to share the work 👍
But the minimum is to at least add a try...except... since it won't work for some Mapping classes (e.g. ChainMap)

In my code, I added an extra if hasattr(mapping, 'clear') and hasattr(mapping, 'update') to support properly defaultdict (e.g. for defaultdict(int), it keeps it instead of switching to defaultdict(None)).
But there are still some cases that are not properly supported like ChainMap.

In my opinion we should probably try to support most common mapping types (stdlib ones) like it is done for Sequence and add a default implementation with

try:
    return type(v)(**result), None
except TypeError:
    # add a warning ?
    return result, None

It would mean that all of this would work

from collections import ChainMap, OrderedDict, defaultdict


class M(BaseModel):
    x: Mapping[str, int]

assert repr(M(x=OrderedDict({1: '3'}))) == "M(x=OrderedDict([('1', 3)]))"

default_d = defaultdict(int)
default_d[1] = '3'
default_d['2']
assert repr(M(x=default_d)) == "M(x=defaultdict(<class 'int'>, {'1': 3, '2': 0}))"

map1 = {1: '3', '2': '4'}
map2 = {'3': 2, 4: '5'}
assert repr(M(x=ChainMap(map1, map2))) == "M(x=ChainMap({'1': 3, 2: '4'}, {3: '2', 4: '5'})"

@ofek WDYT?

@ofek
Copy link
Contributor Author

ofek commented Feb 7, 2021

@PrettyWood That sounds good! If you don't mind though, can you please continue with your approach and open a PR? I trust your knowledge of this codebase much more than mine 🙂

@ofek
Copy link
Contributor Author

ofek commented Feb 7, 2021

@PrettyWood Okay I updated my PR!

@PrettyWood
Copy link
Member

@ofek Thanks! I had a quick look on my side too and updated my code https://github.com/PrettyWood/pydantic/pull/70/files
Maybe you can have a look for inspiration.
I feel like tackling ChainMap is not worth it as it would require a refactoring of the whole _validate_mapping part.
For your mypy issue, I know it's annoying.
Currently it's not possible to do create a TypeVar bound to a Mapping hence my # type: ignore. Don't know if there is a better solution 🤷‍♂️

@laundmo
Copy link
Contributor

laundmo commented Sep 13, 2023

For anyone else stumbling across this issue in search of how to use ChainMap in Pydantic, I implemented it as a custom type which subclasses the stdlib ChainMap

https://gist.github.com/laundmo/909707aa63f8842737af6e7de8f81d0d

I tested that it can:

  • validate from json
  • validate from list of dicts
  • output a json schema of models using this

It's treated just like an array of objects in JSON

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug V1 Bug related to Pydantic V1.X
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants