Update readme #3722
Update readme #3722
Conversation
mattrunyon
added
NoDocumentationNeeded
NoReleaseNotesNeeded
mattrunyon
requested review from
jmao-denver,
rcaudy and
margaretkennedy
as code owners
Co-authored-by: margaretkennedy <82049573+margaretkennedy@users.noreply.github.com>
|
|
||
| ## JS API | ||
|
|
||
| Needs info on how to generate |
There was a problem hiding this comment.
Noting that this and the tests section in the py file still need info.
| venv | ||
| .venv | ||
| *.egg-info |
There was a problem hiding this comment.
Why were these removed? I think the venv is there to handle venvs that pycharm creates, and the egg-info is for python files that are not checked in.
There was a problem hiding this comment.
These were added. And anything in .gitignore without a leading or middle slash will match at any level. So it is effectively the same as **/venv/** or "ignore any folder at any depth called venv". I could actually remove the pyclient/venv since it will match under this new item
| Developers interested in tinkering with and modifying source code should build from the source code. For detailed instructions on how to do this, see [Build and launch Deephaven](https://deephaven.io/core/docs/how-to-guides/launch-build). | ||
| Developers interested in modifying source code should read on for development instructions. |
There was a problem hiding this comment.
The link to the build docs on the website should not have been lost.
https://deephaven.io/core/docs/how-to-guides/launch-build/
| | Package | Version | OS | | ||
| | ------- | ----------------------------- | ------------ | | ||
| | Java | >=11, <20 | All | | ||
| | Python | >= 3.8 | All | | ||
| | Windows | 10 (OS build 20262 or higher) | Only Windows | | ||
| | WSL | 2 | Only Windows | |
There was a problem hiding this comment.
This dependency list does not match what is on the website.
https://deephaven.io/core/docs/how-to-guides/launch-build/
Is python a hard requirement? If not, that is not represented in either place.
There was a problem hiding this comment.
Python isn't a hard requirement. Docker isn't either technically I think.
I'm also not sure the Java version is entirely correct here or the website. I think gradle requires a more specific version to run, but not 100% sure (I had some issues in intellij b/c I had my gradle version set too new)
| @@ -2,6 +2,8 @@ | |||
|
|
|||
|  | |||
|
|
|||
| [Quick start for Docker user guide](https://deephaven.io/core/docs/tutorials/quickstart) | |||
There was a problem hiding this comment.
We should also reference the quick start for pip.
https://deephaven.io/core/docs/tutorials/quickstart-pip/
| @@ -32,269 +34,51 @@ which functions as the data backbone for prominent hedge funds, banks, and finan | |||
|
|
|||
| ## Run Deephaven | |||
|
|
|||
| This section is a quick start guide for running Deephaven from pre-built images. Almost all users will want to run Deephaven using pre-built images. It is the easiest way to deploy. For detailed instructions, see [Launch Deephaven from pre-built images](https://deephaven.io/core/docs/tutorials/quickstart). | |||
| If you want to use Deephaven, see our [Quick start for Docker guide](https://deephaven.io/core/docs/tutorials/quickstart). | |||
There was a problem hiding this comment.
We should also be directing people to https://deephaven.io/core/docs/tutorials/quickstart-pip/. We need to provide very obvious flight paths for docker vs pip users. This document is so lean that there is one flight path for end users.
| For any of the options below, you should first set up a virtual environment in Python. Once created, you must activate the virtual environment every time you open a shell. | ||
|
|
||
| ```sh | ||
| python3 -m venv .venv # Create. Only needed once | ||
| source .venv/bin/activate # Activate the virtual environment | ||
| deactivate # Exit virtual environment when done | ||
| ``` |
There was a problem hiding this comment.
Where does this need to be set up?
| - This is not an exhaustive guide to managing Python environments. | ||
| - Depending on your OS and how your PATH is set up, you may need to use `python3`, or a path to the explicit Python version you want to use. | ||
| - You may set up a "permanent" virtual environment location. | ||
| - You'll need to re-install the wheel whenever you make Python code changes that affect the wheel. | ||
| - `pip` can be a pain if you are trying to (re-)install a wheel with the same version number as before. | ||
| - A `pip install --force-reinstall --no-deps "py/server/build/wheel/deephaven_core-<version>-py3-none-any.whl[autocomplete]"` may do the trick. | ||
| - You can install other Python packages in your venv using `pip install <some-other-package>`. | ||
| - You can set up multiple virtual environments and switch between them as necessary using `source /path/to/other-venv/bin/activate`. | ||
| - You can de-activate the virtual environment by running `deactivate`. | ||
| - You can use the `VIRTUAL_ENV` environment variable instead of sourcing/activating virtual environments: `VIRTUAL_ENV=/my/venv ./gradlew server-jetty-app:run`. |
There was a problem hiding this comment.
This is not thorough enough for an outside user to make progress.
There was a problem hiding this comment.
This document is ultra fragmented and there are not clear thought flows for users to follow. This is more core dump than outside-usable instructions.
There was a problem hiding this comment.
This is ultra stripped down. It needs some more background for outsiders.
| - [Python](./py/README.md) | ||
| - [Web](./web/client-ui/README.md) | ||
| - [Go](./go/README.md) | ||
| - [C++](./cpp-client/README.md) |
| - You may set up a "permanent" virtual environment location. | ||
| - You'll need to re-install the wheel whenever you make Python code changes that affect the wheel. | ||
| - `pip` can be a pain if you are trying to (re-)install a wheel with the same version number as before. | ||
| - A `pip install --force-reinstall --no-deps "py/server/build/wheel/deephaven_core-<version>-py3-none-any.whl[autocomplete]"` may do the trick. |
There was a problem hiding this comment.
Don't we also need to install the py/server-embedded/.../deephaven_server wheel? I've been doing that... but maybe I don't need to. 🤷
There was a problem hiding this comment.
I've found it unnecessary if you're not using the pip installed version of DH (DHaaL I think we called it?)
This section was just moved from the jetty-app readme b/c it didn't make sense there IMO, but it does look like it was just a brain dump to get the info on the page
| - You can install other Python packages in your venv using `pip install <some-other-package>`. | ||
| - You can set up multiple virtual environments and switch between them as necessary using `source /path/to/other-venv/bin/activate`. | ||
| - You can de-activate the virtual environment by running `deactivate`. | ||
| - You can use the `VIRTUAL_ENV` environment variable instead of sourcing/activating virtual environments: `VIRTUAL_ENV=/my/venv ./gradlew server-jetty-app:run`. |
There was a problem hiding this comment.
We should also detail debugging steps somewhere. Just adding -Pdebug allows you to attach a remote debugger and set breakpoints.
|
Superceded by #4209 |
This at least adds some info about development scripts/workflow for Python/Java. Also removes the bulk of the main readme which was a copy our docs quickstart guide
There could still be more improvements such as adding info on how to do testing
Added placeholders for things I know have to be manually generated if there are protobuf changes.