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

Add ExposeMissing context manager #261

Merged
merged 6 commits into from Sep 9, 2020
Merged

Add ExposeMissing context manager #261

merged 6 commits into from Sep 9, 2020

Conversation

lafrech
Copy link
Collaborator

@lafrech lafrech commented Apr 28, 2020

  • Use Python 3.7's context variable feature to create a context manager in which a document returns missing for missing data rather than None
  • Use this context manager in schema_from_umongo_get_attribute

This makes the Document responsible for outputing missing rather than the schema having to guess. Also the schema works with any type of data, not only a Document instance (with other data, the context manger is no-op).

I hoped to remove the whole mongo_world management thanks to this, but there are other issues with data from Mongo vs. Document.

The benefit is not as great as I expected, but I still like this implementation.

@lafrech lafrech added this to the 3.0 milestone Apr 28, 2020
@coveralls
Copy link

Coverage Status

Coverage decreased (-20.9%) to 74.852% when pulling 021a3b3 on expose_missing into 8a0aaf9 on master.

@coveralls
Copy link

coveralls commented Apr 28, 2020

Coverage Status

Coverage increased (+0.06%) to 95.611% when pulling 07f2715 on expose_missing into 53a8c4e on master.

@lafrech
Copy link
Collaborator Author

lafrech commented Apr 29, 2020

This implementation is totally sub-optimal. It misses a nice point of the context variable, which is that we could put the context manager at the dump method level rather than the get_attribute level. This is another reason to like the context_var approach better than the current one.

@lafrech lafrech mentioned this pull request Apr 29, 2020
@lafrech lafrech marked this pull request as draft April 29, 2020 13:33
@lafrech
Copy link
Collaborator Author

lafrech commented Apr 29, 2020

Rebased on master.

  • Now uses the context manager in schema dump method.
  • Removes now useless schema_from_umongo_get_attribute.
  • Exposes (SchemaFomUmongo renamed as) RemoveMissingSchema.
  • Makes RemoveMissingSchema an opt-in. Easy to achieve by passing it to custom BaseDocument and BaseEmbeddedDocument.

@lafrech lafrech marked this pull request as ready for review April 29, 2020 22:43
@lafrech lafrech changed the title Add document.EXPOSE_MISSING context manager Add document.ExposeMissing context manager Sep 7, 2020
@lafrech lafrech changed the title Add document.ExposeMissing context manager Add ExposeMissing context manager Sep 7, 2020
@lafrech lafrech mentioned this pull request Sep 7, 2020
@lafrech
Copy link
Collaborator Author

lafrech commented Sep 9, 2020

Finally, this is the first part of a rework followed in #299 that removes mongo_world and nicely simplifies the code.

@lafrech lafrech merged commit b6f96e5 into master Sep 9, 2020
@lafrech lafrech deleted the expose_missing branch September 9, 2020 09:59
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.

None yet

2 participants