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

Stop repeat inference attempt causing a RuntimeError in Python3.7 #598

Closed
wants to merge 1 commit into from

Conversation

brycepg
Copy link
Contributor

@brycepg brycepg commented Jul 20, 2018

Empty generators and next calls within a generator without a default argument
now cause a cascade effect which results in a RuntimeError.

Raise InferenceError instead of return None to avoid the above problem.

Close pylint-dev/pylint#2317

Empty generators and next calls within a generator without a default argument
now cause a cascade effect which results in a RuntimeError.

Raise InferenceError instead of return None to avoid the above problem.

Close pylint-dev/pylint#2317
@brycepg
Copy link
Contributor Author

brycepg commented Jul 20, 2018

Duplicate CI tasks because I accidentally created a branch on PyCQA/astroid and then deleted

@PCManticore PCManticore self-requested a review July 23, 2018 07:14
@PCManticore
Copy link
Contributor

I'll check this PR these days, thanks! My main concern is why we now need to raise an inference error.

@brycepg
Copy link
Contributor Author

brycepg commented Jul 23, 2018

because returning an empty generator would cause next calls to raise stopiteration which causes a runtimeerror in 3.7. The alternative would be to catch StopIteration on every next call inside of a generator

@PCManticore
Copy link
Contributor

@brycepg The reason I was concerned is that path_wrapper should not care about the fact that in its upper call sites we're raising InferenceError to propagate an inference issue down to the consumers. But from a short look it seems that the corresponding inference methods weren't decorated at all with raise_if_nothing_inferred, which should deal with this problem at a more general level. I've pushed 8841891 which seems to fix the original problem from pylint. I merged your PR locally to get the text without the raising of the InferenceError.

@brycepg
Copy link
Contributor Author

brycepg commented Jul 24, 2018

Alright, sounds good

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