Skip to content

Implementing Generic Structuring Support #51

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

Closed
wants to merge 15 commits into from

Conversation

wakemaster39
Copy link

@wakemaster39 wakemaster39 commented Nov 21, 2018

This is extremely rough and a first pass, it is a proof of concept to see if this is a direction that cattrs is willing to take or not. A working concept with this code can be found below.

from attr import attrs, attrib, make_class
from typing import Dict, Type, Optional, List, TypeVar, Generic
from cattr import structure

@attrs(auto_attribs=True)
class M(Generic[Q]):
    m: Q

@attrs(auto_attribs=True)
class N(Generic[P]):
    n: P

data2 = {
    "n": {
        "m": "test"
    }
}

a = structure(data2, N[M[str]])
print(a)

Right now when when you want to structure a generic it informs you to register a hook. From what I have found, to do this you have to register a call for every instance of your generic and you have to manually define how to build the object.

when you are talking multiple layers down, numerous different instances of the objects, this can become a tedious thing to maintain, especially when you are dealing with "simple" objects in terms of their structuring and destructuring are all ready supported if they were concrete types from the beginning.

To handle generics I tried a couple different ways to implement this as purely an external addon, but I found it was impossible to get it right without injecting code into MultiStrategyDispatch to handle TypeVars.

I also tried to not use lambdas but I found in every function block for the mutlifunction dispatcher I had to insert the:

        if isinstance(cl, TypeVar):
            cl = getattr(mappings, cl.__name__, cl)

This is why I added the lambdas. Mappings was needed in a few handlers list _structure_list which means I had to pass it around to all the different functions so all methods signatures needed to be modified.

make_class was needed because of the lru_cache as all parameters needed to be hashable and this was the best way I could come up with doing it.

This is my attempt to address the problem. This is a breaking change, and I know 1.0.0 is on the horizon, but I personally think this is a worth while change to make. There are a few cleanup tasks that need to be done, documentation would need updating and I have not even looked at the unit tests yet. But I wanted to get some direction on if this is something worth pursuing and could be merged in.

I have only tested on python 3.7 so there is likely some other things that need fixing to support other versions.

@wakemaster39 wakemaster39 mentioned this pull request Nov 23, 2018
@wakemaster39
Copy link
Author

I reworked a few things as I continued testing. My changes broke unstructuring since it used the same multi function dispatching strategy. The changes limit the impact to only structuring now and pretty much limited only to the converters in terms of changes. I think I need to fix converting from tuples still but this is more what I envision would be required long run to implement this feature.

We could in theory not make this a breaking change by inspecting how many arguments the function takes.

Because the converters object is global I don't like the idea of making mapping a property on the object but that would limit the impact to even further to no API change and only internal function changes.

@petergaultney
Copy link
Contributor

I'd be interested in seeing this make it in, though clearly the tests need to be fixed and the use of static typing needs to be changed to be backwards-compatible with old versions of Python.

Is cattrs still maintained, and if so, if I fixed up these tests would this branch be mergeable into a second 1.0 release candidate?

Cameron Hurst added 9 commits November 29, 2019 09:43
There is an issue where when you have nested generics mappings
were being generated when they really needed the upper level
mapping.

@attrs(auto_attribs=True)
class M(Generic[Q]):
    m: Q

@attrs(auto_attribs=True)
class N(Generic[P]):
    n: P

data2 = {
    "n": {
        "m": "test"
    }
}

this provides a good example where the issue can occur
Cameron Hurst added 6 commits December 11, 2019 12:30
Moved away from using dunders where possible and use typing methods
When working with thousands of files, attrs does some stuff
in the background that causes massive iteration looking for a unique
file name. This way we can just cache our results bypassing this
uniqueness
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