-
Notifications
You must be signed in to change notification settings - Fork 7
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
Introduce the concept of "producers" #43
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pmg103
reviewed
Jul 16, 2021
Yeah I think this is an improvement |
pmg103
reviewed
Jul 16, 2021
pmg103
reviewed
Jul 16, 2021
@@ -142,11 +142,11 @@ Related objects can also be produced using the `producers.relationship` function | |||
|
|||
```python | |||
project = projectors.combine( | |||
projectors.wrap_producer("name", producers.attr("name")), | |||
projectors.producer_to_projector("name", producers.attr("name")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this extremely verbose but if its internal only I guess its better to be explicitly very explicit and explicit and verbose and extra verbosely explicit
pmg103
approved these changes
Jul 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a big conceptual shift.
Binding the name to the value by having projectors return dictionaries turned out to not be ideal. It was conflating the lower layers (getting values from instances) with the higher layers (presenting the data) because those projector functions had to give a specific name to the value they returned. This necessitated the introducion of "aliasing" throughout the API, whereby the key returned from the projector was replaced with another one. Aliasing was just a hack around the root problem - the names should only be bound in specs.
This PR introduces the concept of "producers", which are functions which just get a single value from an instance. These are then later wrapped to form projectors.
The recommended way to use
django-readers
is now to write custom producer functions (rather than projector functions), and then mount them explicitly in a spec under a given key:This makes the internals of the library a little bit more complicated, but the end-user experience is actually less confusing. And the only actual breaking-backwards-compatibility change (I think) is the removal of the
alias
functions.