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

PEP 484 type checking #523

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

chadrik
Copy link

@chadrik chadrik commented Sep 29, 2017

Related to: #522

I understand the desire to avoid adding typing as a requirement, but doing so does open up some powerful options that are not otherwise available:

  • Generic classes
  • NamedTuple
  • @overloads

I demonstrated the use of some of these in this commit. Let me know what you think.

@chadrik
Copy link
Author

chadrik commented Oct 29, 2017

Hi, it would be great to have some feedback on this. Namely, whether it is acceptable to add typing as a requirement for python 2.x. Personally, I think we'll be very restricted in what we can accomplish without it.

@lkraider
Copy link
Contributor

I agree with you, adding typing has enough benefits to merit the burden of bringing an extra dependency. Let's move forward with enabling it!

@chadrik
Copy link
Author

chadrik commented Nov 14, 2017

@lkraider glad to hear it.

Question: What is the state of the deprecated mixins? I see in ModelType._export() that a Model class is passed to export_loop(), but export_loop and import_loop behave as if they were passed a Schema instance: they rely on ModelCompatibilityMixin being injected into Model in order to make a Model class appear as if it is a Schema instance.

Should export_loop/import_loop expect Schema, Type[Model] or Union[Schema, Type[Model]]?

@chadrik
Copy link
Author

chadrik commented Nov 14, 2017

Here's the code I'm referring to: https://github.com/schematics/schematics/blob/master/schematics/types/compound.py#L154

You'll notice that ModelType._convert goes through the model class which ultimately ends up passing in its Schema object:

model_class.convert(value, context=context)

... whereas ModelType._export calls directly into export_loop with the Model class instead of the Schema:

export_loop(model_class, value, context=context)

Should this be model_class.export()?

@lkraider
Copy link
Contributor

lkraider commented Nov 14, 2017

I have not yet gone through the compound types. My idea was to phase out the compatibility mixins at the same time move the Model fields into state-machines that keep their own info of whether they are already validated or not. The plan is to free us from having to pass the model around everywhere. But that is still only in the idea phase.

For now, the mixins try to make sure a Model is equivalent to a Schema, eventually we want only the Schema going around.

@chadrik
Copy link
Author

chadrik commented Feb 5, 2018

Hi, I had some time to hack away at this some more. It's going to be a long process, so we should try to break it up into smaller tasks that can be merged as we go. The first of these tasks that I've encountered, which I could use someone else's help on, is converting use of __all__ = module_exports(__name__) into lists of static strings. I know, I know, I'm too lazy to maintain these lists manually, too, but if you use wild imports and __all__, then mypy must be able to statically inspect the names in __all__. Shall I make a separate issue for this?

@lkraider
Copy link
Contributor

lkraider commented Feb 6, 2018

Yes let's do it. I am not a fan of the star import myself, but I think we should curate the exports.

@chadrik
Copy link
Author

chadrik commented Feb 6, 2018

Cool, any chance you can help me out with that? Maybe the simplest thing to do is hack module_exports to pretty-print the list that it finds, then walk through and import all the modules.

@lkraider
Copy link
Contributor

lkraider commented Feb 8, 2018

Sure thing, I plan on working this weekend on the release, and I'll also work on the exports then.

lkraider added a commit that referenced this pull request Feb 15, 2018
@lkraider
Copy link
Contributor

I've made the changes to the master branch, let me know if it works this way.

@lkraider lkraider self-assigned this Mar 3, 2018
@ses4j
Copy link

ses4j commented Jul 10, 2019

Did this die on the vine?

@HJEGeorge
Copy link

Bump - would be very keen on having schematics typing for both schematics models and primitives (some form of typed_dict).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants