Skip to content

Pylint 2 fixes#15487

Merged
balloob merged 4 commits intohome-assistant:devfrom
scop:pylint-2-fixes
Jul 17, 2018
Merged

Pylint 2 fixes#15487
balloob merged 4 commits intohome-assistant:devfrom
scop:pylint-2-fixes

Conversation

@scop
Copy link
Copy Markdown
Member

@scop scop commented Jul 16, 2018

Description:

I'm working on upgrading pylint to 2.0.0. It reports quite a few new things (more on that later in a separate PR), but here are some easliy fixable low hanging fruits.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@scop scop requested review from a team, Danielhiversen, rytilahti and syssi as code owners July 16, 2018 10:56
@ghost ghost added the in progress label Jul 16, 2018
import sys
import threading

from typing import Optional, List, Dict, Any # noqa #pylint: disable=unused-import
Copy link
Copy Markdown
Member

@balloob balloob Jul 16, 2018

Choose a reason for hiding this comment

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

Is this needed?

From PyLint changelog:

Don't emit unused-import anymore for typing imports used in type comments.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it's still needed. Note used in type comments in the pylint changelog -- in this case, Optional, Dict, and Any are not used anywhere. (In my opinion, these cases should be fixed for real by just dropping the actually unused imports, but that's slightly off topic for this particular PR.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that dropping the unused ones should be appropriate. What's the use of importing something and not using it? Isn't that exactly what PyLint is checking?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I suppose it's just an oversight in many places. I'll modify this PR to drop the import-error changes and will take a look at addressing them in a separate one.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've pushed a "Revert pylint 2 inline disable syntax fixes addressing unused-imports" commit to the source branch, but it seems github is having issues at the moment and that commit doesn't show up here yet. Not sure if I should do something about it, but will wait at least a bit first.

@scop scop requested a review from cdce8p as a code owner July 16, 2018 13:44
Will have a go at removing more unused imports altogether first.
@ghost ghost assigned bachya Jul 17, 2018
@balloob balloob merged commit e31dd44 into home-assistant:dev Jul 17, 2018
@ghost ghost removed the in progress label Jul 17, 2018
@scop scop deleted the pylint-2-fixes branch July 17, 2018 18:15
This was referenced Jul 17, 2018
michaeldavie pushed a commit to michaeldavie/home-assistant that referenced this pull request Jul 31, 2018
* pylint 2 inline disable syntax fixes

* pylint 2 logging-not-lazy fixes

* pylint 2 consider-using-in fixes

* Revert pylint 2 inline disable syntax fixes addressing unused-imports

Will have a go at removing more unused imports altogether first.
@balloob balloob mentioned this pull request Aug 3, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
* pylint 2 inline disable syntax fixes

* pylint 2 logging-not-lazy fixes

* pylint 2 consider-using-in fixes

* Revert pylint 2 inline disable syntax fixes addressing unused-imports

Will have a go at removing more unused imports altogether first.
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants