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

Implemented the Table class wrapper, fixes #1486 #1541

Merged
merged 7 commits into from
Nov 23, 2021

Conversation

jmao-denver
Copy link
Contributor

@jmao-denver jmao-denver commented Nov 9, 2021

  1. Wrapped the Table class and those of its methods that are most usable by Python
  2. Added agg.py, constants.py
  3. Added test_table.py, test_table.csv to the test suite
  4. Added Python docstrings for all public methods/properties in the new code
  5. Reorganized the source to make it more pep8 compliant
  6. Fixed a bug in test environment configuration to enable ops on time_tables
  7. Minor changes to the Python client API to keep it consistent with the server package

@jmao-denver jmao-denver added this to the Nov 2021 milestone Nov 9, 2021
@jmao-denver jmao-denver self-assigned this Nov 9, 2021
@jmao-denver jmao-denver changed the title Implemented the Table class wraper, fixes #1486 Implemented the Table class wrapper, fixes #1486 Nov 9, 2021
This is to avoid potential naming conflicts in the future
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.

Quick pass. Didn't pay too much attention to the pydocs.

pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
configs/dh-defaults.prop Outdated Show resolved Hide resolved
pyclient/pydeephaven/_table_interface.py Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Show resolved Hide resolved
pyintegration/deephaven2/__init__.py Outdated Show resolved Hide resolved
pyclient/pydeephaven/_table_interface.py Show resolved Hide resolved
pyintegration/deephaven2/__init__.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/__init__.py Outdated Show resolved Hide resolved
pyintegration/tests/test_table.py Show resolved Hide resolved
pyintegration/tests/test_table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/table.py Outdated Show resolved Hide resolved
pyintegration/deephaven2/combo_agg.py Outdated Show resolved Hide resolved
Got the new agg interface from Ryan and coded against them, however
Ryan's changes haven't landed yet so CI checks are expected to fail
@jmao-denver jmao-denver requested a review from chipkent November 16, 2021 01:34
pyintegration/deephaven2/agg.py Outdated Show resolved Hide resolved
This change is based on feedbacks from devrel. Also to avoid name
shadowing, renamed sum, max, min, count to sum_, max_, min_, count_
pyintegration/deephaven2/agg.py Outdated Show resolved Hide resolved
and restored dh-defaults.prop and changed the aj test cases
@jmao-denver jmao-denver dismissed devinrsmith’s stale review November 23, 2021 21:37

addressed all concerns, the reviewer is OK with merging it to unblock dependent PRs

@jmao-denver jmao-denver merged commit fec97ad into deephaven:main Nov 23, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Nov 23, 2021
@jmao-denver jmao-denver deleted the feature-1486 branch February 8, 2023 18:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hand-wrap the Table class in Python Wrap the Table interface in Python
3 participants