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

drop Field._creation_counter? #49

Open
wbolster opened this issue Mar 8, 2019 · 2 comments
Open

drop Field._creation_counter? #49

wbolster opened this issue Mar 8, 2019 · 2 comments

Comments

@wbolster
Copy link
Contributor

wbolster commented Mar 8, 2019

i admit i do not understand completely what's going on, but the Field._creation_counter seems like a hack to me, and i wonder why it's necessary?

since python 3.6, dictionaries retain insertion order which, iiuc, means the attrs passed to __new__ are in source code order already.

why not assign all fields to __schematype_fields__ instead and use that when ordering is needed?

while looking at it, parent schema fields are included in child schema, but does schematype handle overrides of fields correctly at all? the attrs project has some code that may be relevant to look at here, since it deals with similar problems: https://github.com/python-attrs/attrs/blob/4fe28966e88b9b85c9c2df77ffb34f70175c4492/src/attr/_make.py#L351-L362

@tomchristie
Copy link
Member

since python 3.6, dictionaries retain insertion order which, iiuc, means the attrs passed to new are in source code order already.

Okay... if we can demonstrate in a test case that the field ordering matches the source code ordering, then I'm 100% happy to drop it, yup!

@tomchristie
Copy link
Member

parent schema fields are included in child schema, but does schematype handle overrides of fields correctly at all?

Believe so, yup. Fairly sure there's a test case for exactly that, but worth double checking.

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

No branches or pull requests

2 participants