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

Remove deprecation warnings in test #1416

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kevinjqliu
Copy link
Contributor

@kevinjqliu kevinjqliu commented Dec 7, 2024

Removing deprecation warnings emitted during pytests. This will also prepare us for 0.9.0 release

Fix these errors in make-test:

  • tests/catalog/test_rest.py
    Deprecated in 0.8.0, will be removed in 1.0.0. Iceberg REST client is missing the OAuth2 server URI configuration
  • tests/expressions/test_parser.py::test_is_null
    Deprecated in 0.8.0, will be removed in 0.9.0. Parsing expressions with table name is deprecated. Only provide field names in the row_filter.

@@ -56,7 +56,6 @@ def deprecation_message(deprecated_in: str, removed_in: str, help_message: Optio

def _deprecation_warning(message: str) -> None:
with warnings.catch_warnings(): # temporarily override warning handling
warnings.simplefilter("always", DeprecationWarning) # turn off filter
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing this setting, otherwise there's no way to override a warning

@kevinjqliu kevinjqliu requested a review from Fokko December 13, 2024 16:11
@Fokko
Copy link
Contributor

Fokko commented Dec 20, 2024

@kevinjqliu can you rebase this one? :)

Deprecated in 0.8.0, will be removed in 0.9.0. Parsing expressions with table name is deprecated. Only provide field names in the row_filter.
Deprecated in 0.8.0, will be removed in 1.0.0. Iceberg REST client is missing the OAuth2 server URI configuration
@kevinjqliu kevinjqliu force-pushed the kevinjqliu/remove-deprecation-warnings-in-test branch from d4d9c79 to a5d4d0c Compare December 20, 2024 15:20
@kevinjqliu
Copy link
Contributor Author

done! @Fokko can you take another look? ran locally no warnings in make-test

@@ -70,7 +70,6 @@ def test_equals_false() -> None:
def test_is_null() -> None:
assert IsNull("foo") == parser.parse("foo is null")
assert IsNull("foo") == parser.parse("foo IS NULL")
assert IsNull("foo") == parser.parse("table.foo IS NULL")
Copy link
Contributor

Choose a reason for hiding this comment

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

@kevinjqliu Was this one intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep! its associated with this one

@column.set_parse_action
def _(result: ParseResults) -> Reference:
if len(result.column) > 1:
deprecation_message(
deprecated_in="0.8.0",
removed_in="0.9.0",
help_message="Parsing expressions with table name is deprecated. Only provide field names in the row_filter.",
)
# TODO: Once this is removed, we will no longer take just the last index of parsed column result
# And introduce support for parsing filter expressions with nested fields.
return Reference(result.column[-1])

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