Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix starmap_indexed not passing index #690

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Boudewijn26
Copy link

@Boudewijn26 Boudewijn26 commented Feb 16, 2023

This PR fixes starmap_indexed not actually passing the index to the function

This could be considered a breaking change, although I suspect the impact would be minor. Anyone currently using starmap_indexed must have noticed it's no different from starmap

@@ -65,7 +65,7 @@ def _identity(value: _T1, _: int) -> _T2:

return compose(
ops.zip_with_iterable(infinite()),
ops.starmap_indexed(_mapper_indexed),
ops.starmap(_mapper_indexed),
Copy link
Author

Choose a reason for hiding this comment

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

This depended on the incorrect behavior of starmap_indexed. If it actually were to pass the index, it would pass it twice: once from zip_with_iterable(infinite()) and once from starmap_indexed

@@ -405,6 +405,211 @@ def mapper(x, y):
assert xs.subscriptions == [subscribe(200, 290)]
assert invoked[0] == 3

def test_starmap_with_index_throws(self):
Copy link
Author

Choose a reason for hiding this comment

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

Tests derived from

def test_map_with_index_throws(self):
and adapted for starmap

@Boudewijn26 Boudewijn26 force-pushed the fix/starmap_indexed branch 2 times, most recently from 13100f7 to cab78b4 Compare February 16, 2023 15:30
@coveralls
Copy link

Coverage Status

Coverage: 93.425% (+0.03%) from 93.396% when pulling cab78b4 on Boudewijn26:fix/starmap_indexed into a10cc1c on ReactiveX:master.

@MainRo MainRo force-pushed the fix/starmap_indexed branch from cab78b4 to b6ffaff Compare November 13, 2024 21:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants