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.

Sign up for GitHub

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

Jump to bottom

Update protobuf import to avoid pylint errors #774

Closed
daniel-zullo-frequenz opened this issue Nov 7, 2023 · 3 comments
Closed

Update protobuf import to avoid pylint errors #774

daniel-zullo-frequenz opened this issue Nov 7, 2023 · 3 comments
Assignees
@daniel-zullo-frequenz
Labels
resolution:invalid This doesn't seem right type:enhancement New feature or enhancement visitble to users
Milestone
v1.0.0-rc4

Comments

@daniel-zullo-frequenz
Copy link
Contributor

daniel-zullo-frequenz commented Nov 7, 2023 •
edited by llucax
Loading

What's needed?

Remove the pylint no-name-in-module disable comments in src/frequenz/sdk/microgrid/client/_client.py.

Proposed solution

Update import to avoid the pylint error no-name-in-module as follows:

From:

from google.protobuf.empty_pb2 import Empty  # pylint: disable=no-name-in-module

To:

from google.protobuf import empty_pb2

Once the change above is made, use empty_pb2.Empty instead and remove all the comments to disable the pylint error in src/frequenz/sdk/microgrid/client/_client.py

Additional context

Suggested in #708 (review)
@daniel-zullo-frequenz daniel-zullo-frequenz added part:❓ We need to figure out which part is affected priority:❓ We need to figure out how soon this should be addressed type:enhancement New feature or enhancement visitble to users labels Nov 7, 2023
Merged
@llucax
Copy link
Contributor

llucax commented Nov 7, 2023

I would extend this issue to change all other imports from protobuf stuff, I think there should be more in the client code.

Merged
@llucax
Copy link
Contributor

llucax commented Nov 10, 2023

OK, so it seems the test I did was wrong, and we effectively need to add an ignore where the symbol is used anyways, so I'll close this as invalid.

BTW, the underlying issue seems to be that pylint doesn't read stub (.pyi) files, which is supposed to be fixed in 3.0.0, but doing a simple test it seems like the issue persists with 3.0.0.

I guess we need to test it properly and maybe report to pylint if the issue is still there.

@llucax llucax closed this as not planned Won't fix, can't repro, duplicate, stale Nov 10, 2023
@github-project-automation github-project-automation bot moved this from To do to Done in Python SDK Roadmap Nov 10, 2023
@llucax llucax added resolution:invalid This doesn't seem right and removed part:❓ We need to figure out which part is affected priority:❓ We need to figure out how soon this should be addressed labels Nov 10, 2023
@llucax llucax added this to the v1.0.0-rc4 milestone Nov 10, 2023
github-merge-queue bot pushed a commit that referenced this issue Nov 10, 2023
@daniel-zullo-frequenz
Fix type hint errors in microgrid client (#778)
b49eb89 
The mypy type hint errors in microgrid client were fixed in an attempt
to address #774 which might no longer need to be addressed as the
suggestion is just moving the type hint error from `no-name-in-module`
where `protobuf.*` is imported to `no-member` where it is used. Anyway I
kept these commits in case we want to merge them, otherwise we can just
close this PR and #774.
@llucax
Copy link
Contributor

llucax commented Nov 24, 2023

OK, it seems it is not really fixed in pylint 3, according to this message: pylint-dev/pylint#6281 (comment)

Unfortunately #4987 won't resolve this. #4987 makes it possible to lint .pyi files but it doesn't make it possible to check if a member is defined in a example.pyi file after the member is not found in the example.py file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@daniel-zullo-frequenz daniel-zullo-frequenz

Labels
resolution:invalid This doesn't seem right type:enhancement New feature or enhancement visitble to users
Projects
Python SDK Roadmap
Status: Done
Milestone
v1.0.0-rc4
Development

No branches or pull requests
2 participants
@llucax @daniel-zullo-frequenz