Skip to content

48855 pylint import error #49013

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

Closed

Conversation

slackline
Copy link

@slackline slackline commented Oct 9, 2022

The huge number of errors that were reported by pre-commit that resulted in the disabling of the import-error message in the first place I think stem from the way in which pre-commit was configured to runpylint which was to use the action from the pylint repository. This meant a dedicated virtual environment was used and none of the packages in requirements-dev.txt were installed in the isolated virtual environment that is used (see pre-commit #157). The solution as described in the discussion of that thread (see comment) is to create a local hook and have it use the system environments pylint which is why .pre-commit-config.yaml has been modified.

In addition I have...

  • Explicitly ignored versioneer.py from being linted because it is not a component of pandas and failed linting.
  • Explicitly ignored ǜalidate_min_versions_in_sync.pyas it failed to import key libraries. Its a script used in anotherpre-commitlocal hook that checks minimum version of dependencies are aligned and not a componentpandas`.
  • Added vulture as a required package to requirements-dev.py/ environment.yaml as it was the cause of some pylint import-error.
  • Disabled the pylint import-error around...
    • import zoneinfo (and updated the code that checks for compatability with Python 3.9) in pandas/tests/indexes/datetimes/test_constructors.py.
    • from py.path import local as LocalPath in pandas/tests/io/excel/test_readers.py / pandas/tests/io/pytables/test_read.py / pandas/tests/io/sas/test_sas7bdat.py

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

thanks for your PR

I think we should just not turn off this check, using language: system is unpredictable and I'd rather not risk breaking contributors' workflow

if there was an import error it'd be caught by the tests anyway

@slackline
Copy link
Author

slackline commented Oct 9, 2022

Closing as missing essential pylint package.

Oops, hadn't realised that adding a commit to my fork would make it through to here.

@MarcoGorelli : Ok not a problem, I don't have sufficient knowledge to comment on that. I'll leave it to others to decide.

@slackline slackline closed this Oct 9, 2022
@slackline slackline reopened this Oct 9, 2022
@MarcoGorelli
Copy link
Member

no worries, thanks anyway for your PR - closing then, but there's lot's of other issues to work on if you'd like

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.

2 participants