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

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

Closed
RyanAMoran opened this issue Jul 28, 2016 · 9 comments
Closed

Comments

@RyanAMoran
Copy link

RyanAMoran commented Jul 28, 2016

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.

@hjoukl
Copy link

hjoukl commented Nov 17, 2016

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
Copy link

hjoukl commented Nov 22, 2016

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
Copy link

hjoukl commented Nov 25, 2016

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
Copy link

hjoukl commented Dec 7, 2016

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

@daheise
Copy link

daheise commented Aug 29, 2019

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?

@chocoelho
Copy link
Contributor

@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.

@sbrunner
Copy link
Member

Still valid issue?

@hjoukl
Copy link

hjoukl commented Nov 7, 2024

My old PR has definitely "fallen into the bitbucket". ;-)
I think the problem doesn't exist in modern prospector versions though - haven't checked.

@sbrunner
Copy link
Member

sbrunner commented Nov 7, 2024

Closing, if it occurs again, please reopen it :-)

@sbrunner sbrunner closed this as completed Nov 7, 2024
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

No branches or pull requests

5 participants