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

[#5536] Improvement(client-python): reportUndefinedVariable warning in types.py #6001

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

orenccl
Copy link
Collaborator

@orenccl orenccl commented Dec 26, 2024

What changes were proposed in this pull request?

Update the type variables in the types.py

Why are the changes needed?

Fix: #5536

Does this PR introduce any user-facing change?

No

How was this patch tested?

Unit Test

@orenccl orenccl self-assigned this Dec 26, 2024
@jerryshao
Copy link
Contributor

Is it possible to add a lint check for such kind of Python warning in pyintrc?

@orenccl
Copy link
Collaborator Author

orenccl commented Dec 26, 2024

@jerryshao
I think currently pylint can not detect forward reference
https://peps.python.org/pep-0484/#forward-references

It can detect _instance: BooleanType = None
But can't detect _instance: "BooleanType" = None

@jerryshao
Copy link
Contributor

I see, thanks.

@jerryshao
Copy link
Contributor

Can you please check all the python codes to see if there's similar issue in other files, I think we can fix it all in this PR.

@orenccl
Copy link
Collaborator Author

orenccl commented Dec 27, 2024

Sure, I will update.

@orenccl orenccl marked this pull request as draft December 27, 2024 04:46
@orenccl
Copy link
Collaborator Author

orenccl commented Dec 27, 2024

Hi @xunliu
There are 3 Unresolved reference in client-python.gravitino.catalog.catalog.py

  • Unresolved reference 'TableCatalog'
  • Unresolved reference 'FilesetCatalog'
  • Unresolved reference 'TopicCatalog'

I found FilesetCatalog has usage and it's class is located in gravitino.catalog.fileset_catalog.py

However, I couldn't resolve the other two (TableCatalog and TopicCatalog). Are these using forward references on purpose?

// Has usage and class type
def as_fileset_catalog(self) -> "FilesetCatalog":

// No usage and class type
def as_table_catalog(self) -> "TableCatalog":
def as_topic_catalog(self) -> "TopicCatalog":

Could you help me resolve these warning?

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.

[Improvement] reportUndefinedVariable warning in types.py
2 participants