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

Wrapping RollupTable and TreeTable (2) #3468

Merged
merged 8 commits into from
Mar 3, 2023
Merged

Wrapping RollupTable and TreeTable (2) #3468

merged 8 commits into from
Mar 3, 2023

Conversation

jmao-denver
Copy link
Contributor

Fixes #3249

@jmao-denver jmao-denver added this to the Feb 2023 milestone Mar 1, 2023
@jmao-denver jmao-denver self-assigned this Mar 1, 2023
@jmao-denver jmao-denver changed the title Address seme of the review comments Wrapping RollupTable and TreeTable (2) Mar 1, 2023
@jmao-denver jmao-denver marked this pull request as ready for review March 1, 2023 15:42
jjbrosnan
jjbrosnan reviewed Mar 1, 2023
View reviewed changes
Copy link
Contributor

@jjbrosnan jjbrosnan left a comment

Choose a reason for hiding this comment

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

Comments and questions focused on doc strings. They mostly look good, with some minor changes I've suggested.

jmao-denver and others added 2 commits March 1, 2023 14:25
@jmao-denver @jjbrosnan
Apply suggestions from code review
ba21b0c 
Co-authored-by: JJ Brosnan <84038776+jjbrosnan@users.noreply.github.com>
@jmao-denver
Respond to some review comments
7d6671a
chipkent
chipkent requested changes Mar 2, 2023
View reviewed changes
py/server/deephaven/table.py Show resolved Hide resolved
py/server/deephaven/table.py Show resolved Hide resolved
py/server/deephaven/table.py Outdated Show resolved Hide resolved
py/server/deephaven/table.py Outdated Show resolved Hide resolved
py/server/deephaven/table.py Show resolved Hide resolved
py/server/deephaven/table.py Show resolved Hide resolved
py/server/deephaven/table.py Show resolved Hide resolved
py/server/deephaven/table.py Outdated Show resolved Hide resolved
py/server/deephaven/table.py Outdated Show resolved Hide resolved
jjbrosnan
jjbrosnan previously approved these changes Mar 2, 2023
View reviewed changes
chipkent
chipkent requested changes Mar 3, 2023
View reviewed changes
py/server/deephaven/table.py Outdated Show resolved Hide resolved
rcaudy
rcaudy reviewed Mar 3, 2023
View reviewed changes
py/server/deephaven/jcompat.py Show resolved Hide resolved
py/server/deephaven/table.py Outdated Show resolved Hide resolved
chipkent
chipkent previously approved these changes Mar 3, 2023
View reviewed changes
@jmao-denver @rcaudy
Update py/server/deephaven/table.py
8273b4f 
Co-authored-by: Ryan Caudy <rcaudy@gmail.com>
rcaudy
rcaudy previously approved these changes Mar 3, 2023
View reviewed changes
@jmao-denver jmao-denver requested a review from rcaudy March 3, 2023 18:46
@jmao-denver jmao-denver merged commit b39be21 into deephaven:main Mar 3, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2023
@deephaven-internal
Copy link
Contributor

Labels indicate documentation is required. Issues for documentation have been opened:

How-to: https://github.com/deephaven/deephaven.io/issues/2253
Conceptual: https://github.com/deephaven/deephaven.io/issues/2252
Reference: https://github.com/deephaven/deephaven.io/issues/2251
Blog: https://github.com/deephaven/deephaven.io/issues/2250

@jmao-denver jmao-denver deleted the 3249-rollup-tree-wrapper branch March 31, 2023 03:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rcaudy rcaudy rcaudy approved these changes

@jjbrosnan jjbrosnan jjbrosnan left review comments

@chipkent chipkent Awaiting requested review from chipkent chipkent is a code owner

Assignees

@jmao-denver jmao-denver

Labels
DocumentationNeeded python-server-side ReleaseNotesNeeded Release notes are needed
Projects
None yet
Milestone
Feb 2023
Development

Successfully merging this pull request may close these issues.

Python wrappers for rollups and trees
5 participants
@jmao-denver @deephaven-internal @chipkent @rcaudy @jjbrosnan