Skip to content

fix: type checking #993

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

Merged
merged 10 commits into from
Mar 8, 2025
Merged

fix: type checking #993

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
124d3ea
fix: type checking
chenkovsky Jan 12, 2025
a933fdb
update license
chenkovsky Jan 12, 2025
9897b63
format
chenkovsky Jan 12, 2025
768b6c0
format
chenkovsky Jan 12, 2025
3250848
update catalog
chenkovsky Jan 12, 2025
b165ef3
revert type annotation
chenkovsky Feb 13, 2025
2c8ea03
format
chenkovsky Feb 13, 2025
a7993a1
Merge branch 'main' into feature/type
chenkovsky Feb 16, 2025
0d6cd07
format
chenkovsky Feb 16, 2025
d771d3b
update
chenkovsky Feb 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion python/datafusion/catalog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,10 @@ def __init__(self, table: df_internal.Table) -> None:
"""This constructor is not typically called by the end user."""
self.table = table

@property
Copy link
Contributor

Choose a reason for hiding this comment

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

This is changing the user-facing API, no?

Copy link
Contributor Author

@chenkovsky chenkovsky Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

i checked the test，i think we should use @ property， but previously we didnt do wrapping somewhere， raw table is returned that's why test passed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've become convinced that the current code is broken and that this is the right change. Thank you @chenkovsky for doing this. I do think we need to put some notes to that effect in the section of user facing changes of the PR description.

def schema(self) -> pyarrow.Schema:
"""Returns the schema associated with this table."""
return self.table.schema()
return self.table.schema

@property
def kind(self) -> str:
Expand Down
Loading