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

Fix missing scope_type positional argument for ScopeConsumer #358

Closed
carlio opened this issue Mar 25, 2022 · 7 comments
Closed

Fix missing scope_type positional argument for ScopeConsumer #358

carlio opened this issue Mar 25, 2022 · 7 comments
Assignees
Labels

Comments

@carlio
Copy link
Collaborator

carlio commented Mar 25, 2022

You may see stack-traces containing things like:

    consumer._atomic = ScopeConsumer(new_things, consumer.consumed, consumer.scope_type)  # pylint: disable=W0212
TypeError: <lambda>() missing 1 required positional argument: 'scope_type'

To fix this, pylint-django needs to add backwards/forwards compatibility for ScopeConsumer

See the original issue here - pylint-dev/pylint#5970 (comment)

Pylint 2.13 changed the shape of the ScopeConsumer tuple so pylint-django needs to handle that.

@carlio
Copy link
Collaborator Author

carlio commented Mar 25, 2022

Just an update on this ticket : I have a fix in the master branch, however another change in pylint 2.13 requires some changes to how pylint-django runs tests.

carlio added a commit that referenced this issue Mar 25, 2022
…on a default which is not packaged. Because #358 is imporant, for now pylint-django will just monkey-patch the lint_module_test module to a local test pylintrc
@DanielNoord
Copy link

Just an update on this ticket : I have a fix in the master branch, however another change in pylint 2.13 requires some changes to how pylint-django runs tests.

We should probably package this 😄

Hopefully I remember to do this.

@carlio
Copy link
Collaborator Author

carlio commented Mar 25, 2022

Ok, 2.5.3 is on PyPI now with the fix in despite tox doing its best to make me angry.

@carlio carlio closed this as completed Mar 25, 2022
@Pierre-Sassoulas
Copy link
Member

@DanielNoord would you like me to open an issue ? By packaging do you mean we package the default pylintrc file in pylint.testutil ? We could also add a default value created at runtime.

@carlio
Copy link
Collaborator Author

carlio commented Mar 25, 2022

The specific problem I ran in to was from here - https://github.com/PyCQA/pylint/blob/main/pylint/testutils/lint_module_test.py#L48 - the testing_pylintrc does not exist when using pylint is a dependency.

I'd say that it should be packaged if "LintModuleTest" is considered a public API, but if not then it's an internal thing again and external plugins should handle it themselves; however with some kind of way to do it without monkeypatching the module as I did.

@Pierre-Sassoulas
Copy link
Member

Yeah, I got the idea for the runtime generation from the content of your commit just above that referenced this issue @carlio 😄 (a731230)

@DanielNoord
Copy link

I think it is required as we expose our functional test framework specifically for your use case: allow testing pylint plugins like we test the main checker.

I'm no setup tools expert and don't have much time this weekend, so if somebody finds the time to open a PR please do so.
I think this warrant a new patch release because this will break functional tests for all plugins that use is and update to 2.13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants