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

Bubble up the inner expression #199

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

Conversation

mishamsk
Copy link
Contributor

Hi,

This mostly implements #136 (see below)

Caveats:

  • The codecs code is pretty dense, I've tried to understand it without the comments, but do not feel confident enough to apply any changes there => the new features may or may not work for codecs
  • I dropped the ball on implementing the full path for sequences. I do not think that their going to be any noticeable performance penalty to track current sequence index, but to implement it, the whole unpack_collection must change from an expression generator to a full sub-builder. That's a bit too much for my time constraints;-)

I'd love this to be merged though. I think it adds quite a lot of value already.

Let me know what you think!

@pep8speaks
Copy link

Hello @mishamsk! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 1230:80: E501 line too long (93 > 79 characters)
Line 1239:80: E501 line too long (93 > 79 characters)

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.

2 participants