prospector icon indicating copy to clipboard operation
prospector copied to clipboard

Full set of tools not being run without specifying a file name (Linux)

Open RyanAMoran opened this issue 8 years ago • 7 comments

Per the documention, prospector should be able to run on every file within a directory via a command such as "prospector --strictness veryhigh" where no file name is supplied to run on. This however produces different results than running prospector on a specific file in the same directory via a command such as "prospector --strictness veryhigh python_file.py" . This is the case even if python_file.py is the only file in the directory.

For example, if you had a python_file.py that looked like:

...some code...
...some more code....
print "something to print" #important message to the user........................................................................
bad_tuple = (     1, 2, 3)



def myFunction():
     ....some more code....

When running the command "prospector --strictness veryhigh python_file.py" you get a bunch of pep8 warnings such as "at least two white spaces before inline comment" for line 3 of the file, "function name should be lowercase" for the function, etc. None of these pep8 messages are returned however when running "prospector --strictness veryhigh" from within the directory where python_file.py lives, even if that file is the only thing in the directory.

RyanAMoran avatar Jul 28 '16 21:07 RyanAMoran

I can confirm this behaviour (Python 2.7.5, prospector 0.12.4, pycodestyle 2.0.0)

prospector --profile=/path/to/my/profile.yaml /path/to/my/python_package/python_file.py yields different results than prospector --profile=/path/to/my/profile.yaml /path/to/my/python_package/

Namely all the pep8/pycodestyle messages are missing for the latter.

This is severe, as per routinely checking a whole package you might possily see no errors but per file-based checking (e.g. only the changed files from a VCS changeset) you can suddenly run into loads of errors.

hjoukl avatar Nov 17 '16 09:11 hjoukl

Looks to me like the reason for this is that the pep8/pycodestyle tool only runs on the found packages in case of a directory PATH command line argument:

        # Instantiate our custom pep8 checker.
        self.checker = ProspectorStyleGuide(
            paths=list(found_files.iter_package_paths()),
            found_files=found_files,
            config_file=external_config
        )

Note that only the pylint tool gets "driven" the same way, all the other tools use the iter_module_paths() method.

Update: pylint invocation doesn't suffer from the same problems, though, as all module files get separately added to the files-to-check list by prospector.

Therefore Python files in

  • the root PATH directory
  • non-package directories in PATH

get ignored for pep8/pycodestyle checking.

In case of single file PATHs, iter_package_paths() and iter_module_paths() yield the same results, namely all files given as command line arguments (not even bothering to sort out non-modules).

Python modules in a non-package dir are not uncommon, e.g. in test or scripts directories in source code (repo checkout) trees or in script installation directories.

I'd send a pull request but I'm unsure about the rationale of using iter_package_paths() in pep8/pycodestyle - is this intentional, e.g. due to respecting pycodestyle configuration/exclusions?

hjoukl avatar Nov 22 '16 10:11 hjoukl

Essentially there's 2 ways to fix this:

  1. Use iter_module_paths() instead of iter_package_paths() on the found_files object (which is either a FoundFiles or a SingleFiles instance):
  • Pro: All Python module files are handed over to the pycodestyle checker
  • Pro: finder API unchanged
  • Con: pycodestyle ("external configuration") exclusion rules that target directory names won't work correctly, any more
  1. Use iter_directory_paths() instead of iter_package_paths() on the found_files object:
  • Pro: All Python module-containing directories are handed over to the pycodestyle checker
  • Pro: pycodestyle ("external configuration") exclusion rules that target directory names are respected
  • Con: finder APIs need to be changed:
    • SingleFiles: To hand the same files into the checker, SingleFiles.iter_directory_paths() should just return the file paths as is, not the directories of these files (just what iter_package_paths() does currently).
    • FoundFiles: iter_directory_names() and probably also iter_package_names() need to also yield the root dir path, if it contains modules/packages

hjoukl avatar Nov 25 '16 16:11 hjoukl

445b561 bails out of changing finder APIs or compromising on pycodestyle external configurability and uses a simple 3rd option: 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).

Not overly beautiful implementation-wise due to the need to check if we're dealing with a FoundFiles or SingleFiles instance and behave differently, but alas.

PR: #199

hjoukl avatar Dec 07 '16 13:12 hjoukl

I just ran into this issue on a project. It looks like the PR was good at some point but has fallen into the bitbucket. Is there any news on this bug?

daheise avatar Aug 29 '19 19:08 daheise

@daheise as I'm slowly getting the pace again being away from the project, I'm trying to resolve these old PRs. After I'm done with #308, I'm jumping into #199 and after that, #106.

chocoelho avatar Aug 29 '19 21:08 chocoelho