Fix for pycodestyle not picking up modules in non-package dirs #199
Conversation
Adding tests for the iter_[file|package|directory|module]_paths() methods of FoundFiles and SingleFiles, with an appropriate test data directory tree. Tests the current status quo of method results. Adding myself to CONTRIBUTORS.md.
Fixing the order of the _find_paths() return value assignment to result variables and the parameter order for the FoundFiles initialization to the correct order files, modules, packages, directories. Note that the double error actually evened out i.e. the failure was not visible.
Test expections are set so this tests ok with the current behaviour, for now. This will need to change as pep8 tool ignores python modules in non-package directories when invoked with a single dir PATH command line arg.
…tions. Now set up to fail for the current incorrect behaviour of silently ignoring modules in non-package directories (#179).
Instead of iter_package_paths() use iter_module_paths() for explicit files mode and rootpath for "single directory arg" mode (leaving file lookup to pep8/pycodestyle).
|
Changes Unknown when pulling 9f7e523 on hjoukl:develop into ** on landscapeio:develop**. |
|
S.th. amiss with the code-quality/landscape results reporting? If I follow the Details link everything seems fine, even a slight "score increased" is shown. |
|
Changes Unknown when pulling 0836e8f on hjoukl:develop into ** on landscapeio:develop**. |
This is not necessary here as os.path.abspath() is now ensured in the tool configuration, i.e. by _iter_module_paths() for multiple file PATH args and explicitly for a single dir PATH arg.
|
Changes Unknown when pulling a666605 on hjoukl:develop into ** on landscapeio:develop**. |
|
Fantasic PR and report, thank you very much. I am finally getting to dealing with all the built-up problems so will check this out and merge soon. |
|
Looking forward to this fix. Anything blocking it from being merged? |
|
I'm currently reviewing this PR, just FYI, hope to merge it soon. |
|
Please resolve conflicts. |
|
I won't be able to okay this one it's too technical for me. Please be mindful of your time @hjoukl :) |
|
Hi @carlio, @chocoelho, @Pierre-Sassoulas, happy to see this gain traction again. Alas, this PR has become pretty rusty and at this point I'm unsure how it can be best brought up-to-date. For one I'm unsure if the initial problem reported in #179 persists(?), since 58f9486#diff-e7bec612b6b0afbed5f7116cdb3147775c8262078970c4cddb6d5efe5c0258b7 made significant changes to relevant parts of the affected code. Second, I noticed a big refactoring going on in the develop branch which looks like it makes rebasing quite hard, wrt the finder.py changes and tests. So: What to base this upon in the 1st place - the head of the development branch? What d'ya think? |
I'd at least try to rebase first before re-doing everything, but think you're the one who knows best 😄 |
|
@hjoukl I'd hold off on this for now, as I have a big fat branch with lots of changes including swapping to using |
|
@carlio, @Pierre-Sassoulas Thanks for the infos - feel free to ping me when these |
carlio
added
the
file-finder-rewrite
|
@hjoukl The pathlib changes is now in the develop branch I was able to reproduce your issue, and the new code does not appear to have that problem any more. Please do try it out and if you still have it let me know! I've got a couple of other bugfixes I want to get in before I release this code as an RC. |
|
A follow up, but version 1.8.0 rc0 is now on PyPI |
|
I shall close this because I suspect it is fixed, at least my own tests say it is, but if you try again and find it doesn't work please re-open or create a new issue. |
As reported in #179, the pycodestyle/pep8 tool fails to
when driven by prospector.
See the comments to #179 for more discussion.
This PR takes the simple approach of using found_files.iter_module_paths() for explicit files
mode and rootpath for "single directory arg" mode (leaving file lookup to
pep8/pycodestyle) instead of iter_package_paths().
Comes with extensive tests, also adding a bunch of tests for the general finder functionality.
Edit: PR looks intimidating now judging by the number of commits, but note that the actual changes to the
prospector codebase are small - it's mostly test additions... ;-)