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

Plugin for pandas dataframes and JS API/UI support #1942

Merged
merged 6 commits into from
Apr 27, 2022
Merged

Plugin for pandas dataframes and JS API/UI support #1942

merged 6 commits into from
Apr 27, 2022

Conversation

niloc132
Copy link
Member

@niloc132 niloc132 commented Feb 4, 2022

This is a draft for now, hoping for feedback on how to allow python plugins to be built-in.

Fixes #741

@niloc132 niloc132 added this to the Feb 2022 milestone Feb 4, 2022
devinrsmith
devinrsmith reviewed Feb 7, 2022
View reviewed changes
Copy link
Member

@devinrsmith devinrsmith left a comment

Choose a reason for hiding this comment

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

Think there is room for further discussion in team channels.

docker/server/src/main/docker/Dockerfile Outdated
@@ -20,7 +20,7 @@ RUN set -eux; \

COPY deephaven-wheel/ /deephaven-wheel
RUN set -eux; \
python -m pip install -q --no-index --no-cache-dir /deephaven-wheel/*.whl; \
python -m pip install -q --no-cache-dir /deephaven-wheel/*.whl; \
Copy link
Member

Choose a reason for hiding this comment

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

For final version, we shouldn't be checking in this change. If deephaven-plugin remains external, we'd need to update the deephaven base images as appropriate.

Integrations/python/deephaven/pandas/pandas_as_table.py Outdated
Comment on lines 1 to 3
from deephaven.python_to_java import dataFrameToTable
from deephaven.plugin.object import Exporter, ObjectType
from pandas import DataFrame
Copy link
Member

Choose a reason for hiding this comment

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

We have a choice - this pandas plugin could be completely external (like how the matplotlib plugin lives today). The bad part about that right now is that it has knowledge about "deephaven.python_to_java", and potentially other aspects of Integrations. That said, if we are trying to move to be better "python as a deephaven library", we should be trying to move towards publishing the python integration points as PyPi packages.

Integrations/python/setup.py Outdated
@@ -55,6 +55,12 @@ def normalize_version(ver):
keywords='Deephaven Development',
packages=find_packages(exclude=['docs', 'test']),
install_requires=['deephaven-jpy=={}'.format(__normalized_version__),
'numpy', 'dill>=0.2.8', 'wrapt', 'pandas', 'numba;python_version>"3.0"',
'enum34;python_version<"3.4"'],
'deephaven-plugin==0.0.1.dev5',
Copy link
Member

Choose a reason for hiding this comment

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

If we had deephaven-plugin-pandas external, we could specify it here instead of deephaven-plugin.

Either way, we've tended to put the hard version requirements as part of our docker/**/requirements.txt files as opposed to here in the setup.py.

@niloc132 niloc132 mentioned this pull request Feb 28, 2022
Merged
@pete-petey pete-petey modified the milestones: Feb 2022, Apr 2022 Apr 16, 2022
@niloc132 niloc132 marked this pull request as ready for review April 26, 2022 21:51
devinrsmith
devinrsmith previously approved these changes Apr 27, 2022
View reviewed changes
jmao-denver
jmao-denver previously approved these changes Apr 27, 2022
View reviewed changes
Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

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

maybe we can use a dict for entry_points

@niloc132
Copy link
Member Author

Testing process, run this python code in the java-run server:

import pandas as pd
d = {'IT':[1,20,35,44,51,1000]}
df = pd.DataFrame({'dept': 'IT', 'count': d['IT']})

expected, a table shows up with df as its title, and a bar across it showing that it is a pandas dataframe, with a refresh button to reload the content (for when it is modified directly, without binding a new instance.)

@niloc132 niloc132 dismissed stale reviews from jmao-denver and devinrsmith via 3be7967 April 27, 2022 19:40
@devinrsmith devinrsmith self-requested a review April 27, 2022 20:57
devinrsmith
devinrsmith approved these changes Apr 27, 2022
View reviewed changes
Copy link
Member

@devinrsmith devinrsmith left a comment

Choose a reason for hiding this comment

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

LGTM, assuming you've tested in console with your latest change.

@niloc132 niloc132 merged commit 5826b45 into deephaven:main Apr 27, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jmao-denver jmao-denver jmao-denver approved these changes

@devinrsmith devinrsmith devinrsmith approved these changes

@mofojed mofojed Awaiting requested review from mofojed

Assignees
No one assigned
Labels
jsapi NoDocumentationNeeded plug-ins python
Projects
None yet
Milestone
Apr 2022
Development

Successfully merging this pull request may close these issues.

Display Pandas DataFrames in the Web UI
4 participants
@niloc132 @jmao-denver @devinrsmith @pete-petey