Skip to content

Conversation

@CosmicHorrorDev
Copy link

This PR removes the dependency on itertools in favor of slightly more verbose implementations. The two usages of itertools were for

izip!()

Which was just swapped for chaining .zip() followed by a final .map() to pack the elements into a single tuple

And .group_by()

Which was replaced with grouping everything into a BTreeMap first, then iterating over the key -> group pairs.

This implementation is slightly different since .group_by() will chunk the groups which means that the same key can appear multiple times if it is separated by another key. The implementation in this PR groups all values with the same key regardless of if they are separated by another key, so I'd like for someone else to double check that this still follows intended behavior

@CosmicHorrorDev CosmicHorrorDev changed the base branch from version-0.4 to master November 6, 2022 03:23
@lemmih
Copy link
Collaborator

lemmih commented Apr 2, 2023

Anyone available to double check that this still works as expected?

@CosmicHorrorDev CosmicHorrorDev deleted the remove-dependency-on-itertools branch December 27, 2023 03:57
@briansmith
Copy link

I ended up submitting my own PR to do this, slightly differently. PTAL if you are interested: #876.

@CosmicHorrorDev
Copy link
Author

Super happy to have someone else pick up the torch. Thanks @briansmith !

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