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

minor: Remove deprecated methods #9627

Merged
merged 4 commits into from
Mar 18, 2024
Merged

minor: Remove deprecated methods #9627

merged 4 commits into from
Mar 18, 2024

Conversation

comphead
Copy link
Contributor

Which issue does this PR close?

Closes #.

Rationale for this change

Some code can be removed from DF codebase as its deprecated long time ago

What changes are included in this PR?

Remove deprecated code up to DF 30.0.0 and fixed tests

Are these changes tested?

Yes

Are there any user-facing changes?

@github-actions github-actions bot added logical-expr Logical plan and expressions core Core DataFusion crate labels Mar 15, 2024
@@ -399,20 +374,20 @@ mod test {
}

#[test]
#[allow(deprecated)]
#[ignore = "investigate if this test is needed"]
Copy link
Contributor Author

@comphead comphead Mar 15, 2024

Choose a reason for hiding this comment

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

The method which supposed to replace deprecated one is failing this test, investigating if this test makes sense any more

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at this test and I think it should just be removed (as it is testing a behavior that was removed). The new behavior is covered by

    #[test]
    fn normalize_cols() {

Right above it

Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed a commit to remove the test to your branch

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @comphead -- this has been needed for a long time

@@ -399,20 +374,20 @@ mod test {
}

#[test]
#[allow(deprecated)]
#[ignore = "investigate if this test is needed"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I looked at this test and I think it should just be removed (as it is testing a behavior that was removed). The new behavior is covered by

    #[test]
    fn normalize_cols() {

Right above it

@@ -399,20 +374,20 @@ mod test {
}

#[test]
#[allow(deprecated)]
#[ignore = "investigate if this test is needed"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed a commit to remove the test to your branch

@alamb alamb changed the title minor: Remove deprecated code minor: Remove deprecated methods Mar 16, 2024
@comphead comphead merged commit 4687a2f into apache:main Mar 18, 2024
23 checks passed
Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this pull request May 1, 2024
The suggested replacement `Session::catalog` is already included.

Ref: apache/datafusion#9627
Michael-J-Ward added a commit to Michael-J-Ward/datafusion-python that referenced this pull request May 8, 2024
The method was removed upstream but is used in many tests for `datafusion-python`.

Ref: apache/datafusion#9627
andygrove pushed a commit to apache/datafusion-python that referenced this pull request May 8, 2024
* deps: upgrade datafusion to 37.1.0

* feat: re-implement SessionContext::tables

The method was removed upstream but is used in many tests for `datafusion-python`.

Ref: apache/datafusion#9627

* feat: upgrade dataframe write_parquet and write_json

The options to write_parquet changed.

write_json has a new argument that I defaulted to None. We can expose that config later.

Ref: apache/datafusion#9382

* feat: impl new ExecutionPlanProperties for DatasetExec

Ref: apache/datafusion#9346

* feat: add upstream variant and method params

- `WindowFunction` and `AggregateFunction` have `null_treatment` options.
- `ScalarValue` and `DataType` have new variants
- `SchemaProvider::table` now returns a `Result`

* lint: allow(deprecated) for make_scalar_function

* feat: migrate functions.rs

`datafusion` completed an Epic that ported many of the `BuiltInFunctions` enum to `SclarUDF`.

I created new macros to simplify the port, and used these macros to refactor a few existing functions.

Ref: apache/datafusion#9285

* fixme: commented out last failing test

This is a bug upstream in datafusion

FAILED datafusion/tests/test_functions.py::test_array_functions - pyo3_runtime.PanicException: range end index 9 out of range for slice of length 8

* chore: update Cargo.toml package info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core DataFusion crate logical-expr Logical plan and expressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants