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

Special case __main__ in is_namespace() #1579

Merged
merged 1 commit into from
May 31, 2022

Conversation

jacobtylerwalls
Copy link
Member

Type of Changes

Type
βœ“ πŸ› Bug fix

Related Issue

Glad that this one was not a head scratcher. __main__ is a special case where __spec__ might be None.

Resolves primer failure in pylint. Caused very recently (unreleased) in #1536, so not documented.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2412176420

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 91.928%

Totals Coverage Status
Change from base Build 2411634132: 0.0%
Covered Lines: 9247
Relevant Lines: 10059

πŸ’› - Coveralls

@jacobtylerwalls jacobtylerwalls added this to the 2.12.0 milestone May 31, 2022
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ‘

@DanielNoord
Copy link
Collaborator

Still seeing a crash on pandas:
https://github.com/PyCQA/pylint/runs/6669595502?check_suite_focus=true

Do we need to special case _path__ as well?

@jacobtylerwalls
Copy link
Member Author

I'm trying to see if I can reproduce it locally with just pandas. I think it's possible there's contamination between package names in various primer projects. Instead of cd'ing to each project and linting them separately, we're linting from a cwd that is not a package, just an arbitrary dir. These are edge cases worth fixing, but if pandas lints fine by itself, I just think it's worth observing that these are not "ten thumbs up an hour" kind of cases, since people generally aren't linting their entire venv of packages, unless you're me, and you found #1547 that way :D

@jacobtylerwalls
Copy link
Member Author

See #1581 for the resolution.

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.

4 participants