Why does Sequence try to wrap iterable elements as a new Sequence?

[chrisrink10]

I was messing around with PyFunctional for the first time and it's a great library so thanks for making it!

Issue If you attempt to seq over a sequence, particularly elements which may implement collections.Iterable, you can find yourself in a situation where your sequence elements are returned themselves as Sequences. For transformations, this seems appropriate, but for terminals or actions as I think you call them in the README, I found this behavior to be surprising and counterintuitive.

Here's an trivial example:

>>> seq(("first", ), ("last", )).map(lambda x: x).last()
['last']
>>> type(seq(("first", ), ("last", )).map(lambda x: x).last())
<class 'functional.pipeline.Sequence'>

I would have expected this to return ("last", ) as a tuple, but instead I received a Sequence. Other actions seem to do the same.

I was able to hack around this by using the sequence property of Sequence, but this feels unintuitive to me:

>>> seq(("first", ), ("last", )).map(lambda x: x).sequence[-1]
('last',)
>>> type(seq(("first", ), ("last", )).map(lambda x: x).sequence[-1])
<class 'tuple'>

I am submitting this issue to (a) see if there was a reason behind this behavior in the first place, and (b) if not, figure out what the appropriate resolution would be. I didn't want to jump head first into "fixing" a very intentionally crafted feature 😄so this seemed like the best next step.

If you do agree this is wrong or undesirable, I'm happy to submit a PR to change this behavior, though I'm not exactly sure what other kinds of effects this change might have. If not, I'll consider this a learning opportunity and close this issue!

Thanks!

[EntilZha]

Thanks for opening an issue! The "feature" was intentionally designed, although admittedly its one that I've never been super happy with since it sometimes isn't what you want. Its mostly helpful when you really want to keep on using chained operators even on sub elements, if possible. In the ideal world this would be implemented as something like scala implicits etc, but can't do that in python.

So that being said, I would be open to improving the behavior and/or making it more customizable. I think these changes would be good for a PR:

• In the _wrap code there is handling for not wrapping named tuples (just above where you linked). It probably makes sense to extend this handling to regular tuples as well. If the change could make it still possible for th initial call to seq to use a tuple that would be ideal to minimize breakage.
• Add a option in the Sequence constructor to disable wrapping of subsequences. This way at the very least there is a way to avoid it entirely.

With regards to actions, I hadn't considered before deciding to wrap/not wrap based on that so I'll give it more thought. My main concern would be API breakage so would need to be careful.

[chrisrink10]

Thanks for the quick response!

Those were both ideas I'd considered when I was submitting this issue, so I'm glad I wasn't too far off from what you were thinking. Let me try to put a PR together in the next few days which addresses those points and we can chat about it in there.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.