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

Add PartitionedTable transform() and partitioned_transform() methods #2557

Merged
merged 18 commits into from
Jun 23, 2022
Merged

Add PartitionedTable transform() and partitioned_transform() methods #2557

merged 18 commits into from
Jun 23, 2022

Conversation

jmao-denver
Copy link
Contributor

Fixes #2549

@jmao-denver jmao-denver added this to the Jun 2022 milestone Jun 17, 2022
@jmao-denver jmao-denver self-assigned this Jun 17, 2022
rcaudy
rcaudy reviewed Jun 22, 2022
View reviewed changes
py/server/deephaven/table.py Outdated Show resolved Hide resolved
py/server/deephaven/table.py Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonFunction.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonBiFunction.java Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonFunction.java Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonObjectWrapper.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonObjectWrapper.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonValueGetter.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonValueGetter.java Outdated Show resolved Hide resolved
jmao-denver and others added 5 commits June 23, 2022 09:24
@jmao-denver @rcaudy
Update py/server/deephaven/table.py
7e105a8 
Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update py/server/deephaven/table.py
d4f8e69 
Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
2649b54 
…thonObjectWrapper.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
95548c7 
…thonValueGetter.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver
Respond to review comments
c89d009
@jmao-denver jmao-denver requested a review from rcaudy June 23, 2022 20:03
rcaudy
rcaudy reviewed Jun 23, 2022
View reviewed changes
Integrations/src/main/java/io/deephaven/integrations/python/PythonBiFunction.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonBiFunction.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonBiFunction.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonFunction.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonFunction.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonFunction.java Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonBiFunction.java Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonBiFunction.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonBiFunction.java Outdated Show resolved Hide resolved
Integrations/src/main/java/io/deephaven/integrations/python/PythonFunction.java Outdated Show resolved Hide resolved
rcaudy
rcaudy reviewed Jun 23, 2022
View reviewed changes
py/server/deephaven/table.py Outdated Show resolved Hide resolved
py/server/deephaven/table.py Outdated Show resolved Hide resolved
jmao-denver and others added 8 commits June 23, 2022 14:51
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
719d659 
…thonBiFunction.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
77ec5ab 
…thonBiFunction.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
dc0f3e4 
…thonBiFunction.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
00a16a3 
…thonFunction.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
4d2523d 
…thonFunction.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
d86a1aa 
…thonFunction.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Update Integrations/src/main/java/io/deephaven/integrations/python/Py…
8e92cc6 
…thonBiFunction.java

Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
@jmao-denver @rcaudy
Apply suggestions from code review
80ac761 
Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
jmao-denver and others added 2 commits June 23, 2022 15:02
@jmao-denver jmao-denver merged commit 2d04563 into deephaven:main Jun 23, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 2022
chipkent
chipkent reviewed Jun 24, 2022
View reviewed changes
Integrations/src/main/java/io/deephaven/integrations/python/PythonFunction.java
return PythonValueGetter.getValue(out, classOut);
}

public static class PythonUnaryOperator<T> extends PythonFunction<T, T> implements UnaryOperator<T> {
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it should be a separate file / class and not a subclass.

Integrations/src/main/java/io/deephaven/integrations/python/PythonValueGetter.java
if (valueIn == null) {
return QueryConstants.NULL_FLOAT;
}
return (float) valueIn.getDoubleValue(); // NB: should there be a getFloatValue() in jpy?
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to error on casting out of range? Handle nulls in cast?

Integrations/src/main/java/io/deephaven/integrations/python/PythonValueGetter.java
if (valueIn == null) {
return QueryConstants.NULL_SHORT;
}
return (short) valueIn.getIntValue(); // NB: should there be a getShortValue() in jpy?
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to error on casting out of range? Handle nulls in cast?

Integrations/src/main/java/io/deephaven/integrations/python/PythonValueGetter.java
if (valueIn == null) {
return QueryConstants.NULL_BYTE; // NB: should there be a getByteValue() in jpy?
}
return (byte) valueIn.getIntValue();
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to error on casting out of range? Handle nulls in cast?

py/server/deephaven/table.py
Comment on lines +1477 to +1481
"""Join the underlying partitioned Tables from this PartitionedTable and other on the key columns, then apply
the provided function to all pairs of constituent Tables with the same keys in order to produce a new
PartitionedTable with the results as its constituents, with the same data for all other columns in the
underlying partitioned Table from this. Note that if the Tables underlying this PartitionedTable or other
change, a corresponding change will propagate to the result.
Copy link
Member

Choose a reason for hiding this comment

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

    """Join this and the other PartitionedTables on the key columns and applies the provided function to all pairs
    of constituent tables with the same keys.  The resulting tables are used to produce a new PartitionedTable 
    with the results as its constituents and with the same data as this PartitionedTable for all other columns.

    Note that if the Tables underlying this PartitionedTable or other change, a corresponding change will 
    propagate to the result.

@jmao-denver jmao-denver deleted the 2549-partitioned-table-transform branch February 8, 2023 18:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chipkent chipkent chipkent left review comments

@rcaudy rcaudy rcaudy approved these changes

Assignees

@jmao-denver jmao-denver

Labels
DocumentationNeeded python-server-side
Projects
None yet
Milestone
Jun 2022
Development

Successfully merging this pull request may close these issues.

Add PartitionedTable transform methods to the Python wrapper
3 participants
@jmao-denver @chipkent @rcaudy