Add examples

[elijahpetty]

The accompanying doc will be in a deephaven-core PR.

[Copilot]

Pull request overview

This PR adds example code demonstrating three Python packaging scenarios for Deephaven applications: a library-only package, a CLI-only package, and a combined package. The examples show how to structure Python packages using modern packaging standards with pyproject.toml, implement CLI tools with Click, and create reusable Deephaven query functions.

Changes:

• Added three complete example packages (my_dh_library, my_dh_cli, my_dh_toolkit) demonstrating different packaging approaches
• Included sample CSV data files for testing the examples
• Updated main README with comprehensive documentation on package structure, usage patterns, and troubleshooting

Reviewed changes

Copilot reviewed 34 out of 38 changed files in this pull request and generated 29 comments.

Show a summary per file

File	Description
my_dh_library/*	Library-only package with reusable Deephaven query and utility functions
my_dh_cli/*	CLI-only package with command-line tools for CSV processing
my_dh_toolkit/*	Combined package with both library functions and CLI tools
data/*	Sample CSV files for testing the example packages
README.md	Comprehensive documentation covering all three packaging scenarios and usage examples

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[chipkent]

Wording here is weird. Is "your" really the proper voice for this stuff? Assess the rest of the document for voice.

[chipkent]

Duplicates ./data/batch/*

[chipkent]

Duplicates ./data/batch/*

There are a ton of these duplications. I'm not commenting further.

[chipkent]

Until these are fixed, the signal to noise is too bad for me to review further.

[Copilot]

🟡 Changes recommended

The code claims Python 3.8 support but uses list[str] annotations that break at runtime on Python 3.8 without postponed evaluation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 36/37 changed files
• Comments generated: 2
• Review effort level: Lite

[chipkent]

In reviewing this, I've had some confusion. This file looks like a concatenation of the prior files. I'm not sure if this is intentional, confusing, or just doesn't matter. I haven't read enough of the code to have a view. Just noting it here.

[chipkent]

In my quick scan of the code and the readme, I'm having a hard time figuring out what the story is. It seems like examples are doing multiple things at once, instead of each illustrating a single concept. The readme descriptions of the example then doesn't enlighten the case much.

Below is my chat with Copilot about this:

===

What is this readme trying to show? The illustrative examples seem garbled and unclear and like each does multipel things -- just making them confusing. Are they confusing?

===

Yes — looking at this diff, the README is trying to do too much in each section, and it does make the examples harder to follow. A few concrete problems:

1. Each "Try the X package" section mixes multiple execution modes without clearly separating them

For example, the CLI-only section shows, in sequence:

installing the package
running it as an installed shell command (my-dh-query)
then also importing the underlying function directly in Python (from my_dh_cli.cli import my_dh_query)
These are two different usage patterns (CLI invocation vs. programmatic call) bolted together under one "Try it" header, with only a short transition sentence ("The underlying function is also importable...") separating them. A reader skimming for "how do I just run this from the terminal" has to wade through Python import code too.

2. The combined package section is the worst offender

It shows, back-to-back: two CLI commands (my-dh-toolkit-query, my-dh-toolkit-process), then a full server-startup snippet, then library imports, then calling the same underlying functions two different ways (from my_dh_toolkit.queries import ... and from my_dh_toolkit import my_dh_query, batch_process). That's four different ways to do roughly two things, all in one code block region. It's demonstrating flexibility, but at the cost of a clear "here's the one obvious way to use this" path.

3. Repeated boilerplate obscures the actual point of each example

The Server(port=10000, jvm_args=["-Xmx4g"]).start() snippet is repeated near-verbatim in almost every section. That's necessary context, but it dilutes the examples — the reader has to mentally filter out the same 3 lines every time to find what's actually different about that example.

4. Structural/informational overlap

There's now a "Packaging scenarios" section (when to use library-only/CLI-only/combined) and a separate "Execution patterns" section (entry points vs. module execution) and per-package "Usage" snippets at the top and "Try the X package" walkthroughs further down. That's four different places explaining overlapping concepts (when/how to use each package type), which is a lot for a reader to reconcile into one mental model.

Net assessment: The content itself is accurate and reasonably complete, but the presentation conflates "here's the simplest way to use this" with "here's every way you could possibly use this." A cleaner version would pick one primary usage pattern per package type for the main walkthrough (e.g., just the CLI command) and move the "you can also import the underlying function" variant into a clearly separate, secondary callout — rather than interleaving them as equally-weighted steps.

If you'd like, I can draft a tightened version of one of these sections (e.g., the combined package walkthrough) to show the difference concretely.

[chipkent]

Copilot reviw for the repo was set at a low / cheap level. I increased the level and reran it.

[Copilot]

🟡 Changes recommended

Documentation includes non-working import and src-layout instructions, while batch CSV reads bypass intended CLI error handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (5)

Previously missed (4) — in code that hasn't changed since the last review.

my_dh_toolkit/src/my_dh_toolkit/processor.py:33

• Unlike the single-file command, the batch command does not translate CSV read failures into ClickException. An unreadable or malformed file therefore escapes the CLI as a raw exception/traceback instead of a contextual command error; wrap this read consistently with my_dh_query.
  README.md:232
• With the documented src/ layout, the project root does not place src on sys.path, so this command fails with No module named my_package unless the package is installed (or PYTHONPATH is changed). Describe regular/editable installation as a prerequisite rather than claiming installation is unnecessary.

This issue also appears on line 320 of the same file.
setuptools-deployment.md:79

• The 4GB heap is not a default here; the preceding example explicitly requests it with -Xmx4g. Calling it the default can mislead users sizing deployments, so describe it as this example's configured maximum heap.
  setuptools-deployment.md:531
• The documented batch implementation omits the CSV-read error handling used by the single-file implementation, so users following this guide get a raw traceback for unreadable or malformed input. Keep the guide synchronized with the corrected package code by wrapping this call in ClickException.

README.md:323

• This repeats the unsupported no-install workflow for a src/-layout package. Running from the project root cannot discover my_package; update the heading and benefits to reflect that a regular or editable installation is required.

**Run without installation:**
```shell
python -m my_package

- **Files reviewed:** 24/25 changed files
- **Comments generated:** 1
- **Review effort level:** Balanced
</details>

[chipkent]

@copilot I want you to rereivew this changeset. This repo is intended to illustrate different ways that deephaven packages can be installed. Each example should be clear and provide an easy to understand story. The readme must be best practices user documentation showing users how to use the examples and modify the examples for their own use. Your prior review was of low quality and did not address any of these ultra important topics.

[Copilot]

@copilot I want you to rereivew this changeset. This repo is intended to illustrate different ways that deephaven packages can be installed. Each example should be clear and provi...

Re-reviewed the full changeset against the three goals and fixed everything found, in bc71cf1. I validated by installing all three packages and running every documented command and snippet end-to-end against a live Deephaven server.

Findings and fixes:

1. Examples didn't each tell one story. The CLI example's docs also taught importing its internals, and the toolkit section showed four usage paths for two interfaces. Now: my_dh_library = install → import; my_dh_cli = install → run the command; my_dh_toolkit = one library path + one CLI path. The "call the underlying CLI functions from Python" pattern is gone everywhere.

2. End-to-end testing exposed a real design bug. my_dh_toolkit/__init__.py exported the CLI functions (the source of the confusing duplicate API) — and switching it to export the library API broke both entry-point commands, because importing deephaven requires a running server. The correct combined-package structure is a minimal, import-light __init__.py with the library API in queries/utils submodules. That's now the code, and the README calls it out as the key structural lesson of the combined example.

3. README wasn't usable as user documentation. Rewrote it: a table of the three patterns up front, a "Choose an example" guide, per-example sections (story → structure → validated try-it → what to study), and a new "Adapt an example for your own project" section with the copy/rename/entry-point steps and the three names that must stay in sync. All commands run from the repo root and were verified verbatim. Removed the three overlapping concept sections; voice is now consistent (imperative for instructions, descriptive for the examples).

4. Data confusion. data/batch/ was sample.csv split into three files. The batch files now contain distinct rows, and the README documents which example uses which input.

5. Consistency/correctness. setuptools-deployment.md synced with the code (including the missing read_csv import), and batch_process now reports unreadable CSVs via ClickException like the query command does.

[chipkent]

@copilot address all of the comments

[Copilot]

@copilot address all of the comments

Addressed all six comments in dfab129:

• Example 3 heading — applied your suggestion verbatim, and aligned "command line" wording across the docs.
• Example 3 clarity — rewrote the story, structure annotations, and try-it text so the section reads as one idea: both interfaces in one package, reusing the code from Examples 1 and 2.
• Unclear __init__.py bullet — now walks through the reasoning step by step and contrasts with my_dh_library.
• Document organization — Sample data moved up to follow "Get the examples", before any example uses it.
• pyproject.toml completeness — verified every import maps to a declared dependency; deephaven-server provides the deephaven module through its pinned deephaven-core dependency, now documented with comments in each toml.
• setuptools-deployment.md — audited against the repo: all code listings synced, and inaccurate claims fixed (src-layout link, JVM memory, per-example sample data).

Validated by reinstalling all three packages in a clean venv (pip check clean) and running every documented command and snippet end-to-end against a live Deephaven server.

[chipkent]

This is still a complex thing. Are we showing them best practices? Can we do something better and more simple?

[chipkent]

I'm running out of energy and focus. This file has not been reviewed or checked for consistency vs the rest of the repo and PR.

[chipkent]

@copilot Carefully review the file.

[Copilot]

Reviewed and corrected the guide in 7173ab4. I fixed the src-layout reference, aligned the toolkit batch command example with the repository's data/batch/ input, and made the related-doc links match the topics they name.

[Copilot]

🟡 Changes recommended

Batch processing can reprocess generated outputs, and the guide conflicts with the stated PR scope.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 24/25 changed files
• Comments generated: 3
• Review effort level: Balanced