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

[bugfix] Find files with ./ as input with a __init__.py file #9211

Merged
merged 6 commits into from
Dec 7, 2023
Merged

Conversation

CareF
Copy link
Contributor

@CareF CareF commented Oct 31, 2023

Type of Changes

Type
🐛 Bug fix

Description

When working as pylint --recursive=y ./ or pylint --recursive=y . on a directory with a __init__.py file, pylilnt will not be able to walk through other files due to an empty input for modutils.get_module_files because os.path.dirname("__init__.py") == "". This PR tries to fix this behavior.

Closes #9210

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #9211 (d804da2) into main (b0db61e) will increase coverage by 0.01%.
Report is 17 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #9211      +/-   ##
==========================================
+ Coverage   95.79%   95.81%   +0.01%     
==========================================
  Files         173      173              
  Lines       18714    18762      +48     
==========================================
+ Hits        17927    17976      +49     
+ Misses        787      786       -1     
Files Coverage Δ
pylint/lint/expand_modules.py 95.23% <100.00%> (+0.05%) ⬆️

... and 7 files with indirect coverage changes

This comment has been minimized.

This comment has been minimized.

@CareF
Copy link
Contributor Author

CareF commented Dec 6, 2023

Hi @Pierre-Sassoulas , anything I can do to improve this?

@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.0.3 milestone Dec 6, 2023
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.

Sorry for not being very reactive on this one. MR that modify file discovery are really hard to review and can lead to issues with namespace package or the like so I'm very afraid to accept them 😄 This however LGTM because the only change is easy to understand os.path.dirname("__init__.py") == "" and everything else is doc or test (I did not realize that when skimming). Great MR !

tests/lint/unittest_expand_modules.py Outdated Show resolved Hide resolved

This comment has been minimized.

Comment on lines 1 to 2
Fix bug for not being able to walk through files when work on `./` as input
at a directory with a `__init__.py` file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This sentence is not really understandable. I'd like to suggest something to improve but I don't even really know what it is meant to say.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I'm not English native. What I mean here is that:

  • It's a bug fix
  • The current situation is that, if there is a __init__.py file in a directory, pylint is not able to walk (iteratre through) the files in this directory

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

github-actions bot commented Dec 7, 2023

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit d804da2

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.

Than kyou for reviewing @DanielNoord ! Merging because the warning in readthedoc is unrelated and fixed on the main branch.

@Pierre-Sassoulas Pierre-Sassoulas merged commit abdb874 into pylint-dev:main Dec 7, 2023
43 of 44 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 7, 2023
Co-authored-by: Pierre Sassoulas <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
(cherry picked from commit abdb874)
@Pierre-Sassoulas
Copy link
Member

Congratulation on becoming a pylint contributor @CareF :)

Pierre-Sassoulas pushed a commit that referenced this pull request Dec 7, 2023
…h a __init__.py file (#9286)

* [bugfix] Find files with ./ as input with a __init__.py file (#9211)

Co-authored-by: Pierre Sassoulas <[email protected]>
Co-authored-by: Daniël van Noord <[email protected]>
(cherry picked from commit abdb874)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--recursive fail to parse folder with __init__.py at the root when using relative path
3 participants