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

Concat map #678

Merged
merged 8 commits into from
Dec 30, 2022
Merged

Concat map #678

merged 8 commits into from
Dec 30, 2022

Conversation

matiboy
Copy link
Collaborator

@matiboy matiboy commented Dec 26, 2022

@coveralls
Copy link

coveralls commented Dec 29, 2022

Coverage Status

Coverage: 93.396% (+0.02%) from 93.375% when pulling c2ad7fe on matiboy:concat-map into 21ba043 on ReactiveX:master.

Copy link
Collaborator

@dbrattli dbrattli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matiboy Thanks for adding this. Looks great!

@dbrattli
Copy link
Collaborator

dbrattli commented Dec 30, 2022

One thing I suddenly realized is that we might not need the _concat_map file if we make the composition inside __init__.py e.g:

return compose(map(project), merge(max_concurrent=1))

@matiboy
Copy link
Collaborator Author

matiboy commented Dec 30, 2022

One thing I suddenly realized is that we might not need the _concat_map file if we make the composition inside __init__.py e.g:

return compose(map(project), merge(max_concurrent=1))

Sorry mis-read initially and kept the "verbose" definition. But yes, beautiful one-liner.

@dbrattli
Copy link
Collaborator

Looks great. PS: another change we want to do is to borrow the curry_flip function from the Expression library to simplify all the operator implementations by removing all the nested functions. Take a look at https://github.com/cognitedata/Expression/blob/main/expression/collections/block.py#L602 Thus you can put source back as the first parameter, and curry-flip will move it as the last parameter curried (returning a function that takes it as the parameter). I can open an issue to discuss this more.

@dbrattli dbrattli merged commit d0c24d6 into ReactiveX:master Dec 30, 2022
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.

3 participants