-
-
Notifications
You must be signed in to change notification settings - Fork 143
Adds transducers support #904
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
Conversation
@sobolevn
sobolevn
left a comment
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.
Great work!
returns/transducers/tfilter.py
Outdated
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.
Let's import reduce from functools, because user might be confused: is it our reduce or default one?
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.
We need to use our reduce since we have more features than the built-in reduce
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.
We tend to use decorator / factory naming when defining nested decorators
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.
Yep, but I think using that pattern might be weird:
def tmap(function): def reducer(step): def map_(acc, value): ... return map_ return reducer def tmap(function): def decorator(step): def factory(acc, value): ... return map_ return reducer
In fact transducers is not intended to be use as a decorator and I think tmap -> reducer -> map_ is more meaningful! 🤔
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.
Let's make the first line more user-friendly by removing :py:func: stuff.
It can be later in the body.
120a9fa to
c6b4222
Compare
c6b4222 to
f589867
Compare
@sobolevn
sobolevn
left a comment
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.
Great work 👏
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.
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.
And all other tests as well.
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.
Should we? Raising exceptions is not something we do in this library (at least we try to).
Can we just return the empty sequence? Or do anything else?
I have made things!
Checklist
CHANGELOG.mdRelated issues
🙏 Please, if you or your company finds
dry-pythonvaluable, help us sustain the project by sponsoring it transparently on https://github.com/sponsors/dry-python. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.