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

False positive implicit yield violation #1057

Closed
AlwxSin opened this issue Dec 5, 2019 · 7 comments · Fixed by #1062 or #1091
Closed

False positive implicit yield violation #1057

AlwxSin opened this issue Dec 5, 2019 · 7 comments · Fixed by #1062 or #1091
Labels
bug Something isn't working documentation Docs related task help wanted Extra attention is needed level:starter Good for newcomers pr-merged

Comments

@AlwxSin
Copy link
Contributor

AlwxSin commented Dec 5, 2019

Bug report

What's wrong

With common function to split iterable by chunks WPS raises ImplicitYieldFromViolation

def chunks(value, chunk_size):
    """Yield successive n-sized chunks from l."""
    for index in range(0, len(value), chunk_size):
        yield value[index:index + chunk_size]

How is that should be

ImplicitYieldFromViolation should be raised only if no other operations with iterable item.

@AlwxSin AlwxSin added the bug Something isn't working label Dec 5, 2019
@sobolevn
Copy link
Member

sobolevn commented Dec 5, 2019

What about

yield from (
    value[index:index + chunk_size]
    for index in range(0, len(value), chunk_size)
)

?

@AlwxSin
Copy link
Contributor Author

AlwxSin commented Dec 6, 2019

Less readable in my taste. But if no other choices I will use it. Thanks.

@sobolevn
Copy link
Member

sobolevn commented Dec 6, 2019

Thanks for raising this, I guess we need to update the violation docs about this case.

@sobolevn sobolevn added documentation Docs related task help wanted Extra attention is needed level:starter Good for newcomers labels Dec 6, 2019
@sobolevn sobolevn added this to the Version 0.13.x milestone Dec 6, 2019
andreaestrada added a commit to andreaestrada/wemake-python-styleguide that referenced this issue Dec 8, 2019
Address issue wemake-services#1057. Update violation docs to make it easier for developers to understand how to refactor more complex yield statements.
andreaestrada added a commit to andreaestrada/wemake-python-styleguide that referenced this issue Dec 8, 2019
Address issue wemake-services#1057. Update violation docs to make it easier for developers to understand how to refactor more complex yield statements.
andreaestrada added a commit to andreaestrada/wemake-python-styleguide that referenced this issue Dec 8, 2019
Address issue wemake-services#1057. Update violation docs to make it easier for developers to understand how to refactor more complex yield statements.
@helpr helpr bot added the pr-available label Dec 9, 2019
sobolevn pushed a commit that referenced this issue Dec 9, 2019
* Update violation docs for yield from violation.

Address issue #1057. Update violation docs to make it easier for developers to understand how to refactor more complex yield statements.

* Fix error message for WPS349

Fix issue #1044.

* Update CHANGELOG.md

* Update refactoring.py
@helpr helpr bot added pr-merged and removed pr-available labels Dec 9, 2019
@AlwxSin
Copy link
Contributor Author

AlwxSin commented Dec 24, 2019

@sobolevn there is still a bug.
Python does not support 'yield from' inside async functions

@sobolevn
Copy link
Member

Yes, that's true.

@sobolevn sobolevn reopened this Dec 24, 2019
@sobolevn
Copy link
Member

This rule should be ignored for async functions.

Can you please add a check that if isinstance(get_context(node), ast.AsyncFunctionDef): return?

@AlwxSin
Copy link
Contributor Author

AlwxSin commented Dec 24, 2019

Of course, I'll do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Docs related task help wanted Extra attention is needed level:starter Good for newcomers pr-merged
Projects
None yet
2 participants