Skip to content

Conversation

@gvwilson
Copy link
Contributor

@gvwilson gvwilson commented Nov 29, 2024

Note: fails some tests - see below.

  1. Modify ./README.md and copy it into ./src/py/README.md.
    • Because Python packaging won't let me include ../../README.md in pyproject.toml.
  2. Add diagram in assets/architecture.svg showing the old and new designs.
    • Created with the desktop version of draw.io.
  3. Rename src/py/kaleido as src/py/kaleido2.
  4. Modify src/py/pyproject.toml to rename package.
  5. Edit src/py/kaleido2/__init__.py to change kaleido to kaleido2.
    • FIXME: I have left environment variables and JS stuff as kaleido - should this change?
  6. Edit src/py/kaleido2/scopes/plotly.py to refer to kaleido2 instead of kaleido.

After doing all of this, go into src/py and:

  1. uv venv and activate the virtual environment (using Python 3.12.5).
  2. uv pip install -e . to create editable install.
  3. uv pip install plotly pandas so that manual tests will run.
  4. python tests/manual.py to run manual tests.

Output is as shown below: 7 tests fail with the same error; I presume we're going to have to make changes to plotly.py to pick up kaleido2.

python tests/manual.py
Done!
Successes: 28/35
Errors:
[
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n')
]
Please send over test-results.zip
Logs and images in ./test-results/

test-results.zip

Note: fails some tests - see below.

1.  Modify `./README.md` and copy it into `./src/py/README.md`.
    -   Because Python packaging won't let me include `../../README.md` in `pyproject.toml`.
2.  Add diagram in `assets/architecture.svg` showing the old and new designs.
    -   Created with the desktop version of [draw.io](https://app.diagrams.net/).
3.  Rename `src/py/kaleido` as `src/py/kaleido2`.
4.  Modify `src/py/pyproject.toml` to rename package.
5.  Edit `src/py/kaleido2/__init__.py` to change `kaleido` to `kaleido2`.
    -   FIXME: I have left environment variables and JS stuff as `kaleido` - should this change?
6.  Edit `src/py/kaleido2/scopes/plotly.py` to refer to `kaleido2` instead of `kaleido`.

After doing all of this, go into `src/py` and:

1.  `uv venv` and activate the virtual environment (using Python 3.12.5).
2.  `uv pip install -e .` to create editable install.
3.  `uv pip install plotly pandas` so that manual tests will run.
4.  `python tests/manual.py` to run manual tests.

Output is as shown below: 7 tests fail with the same error;
I presume we're going to have to make changes to plotly.py to pick up `kaleido2`.

```
python tests/manual.py
Done!
Successes: 28/35
Errors:
[
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n'),
  ValueError('\nImage export using the "kaleido" engine requires the kaleido package,\nwhich can be installed using pip:\n    $ pip install -U kaleido\n')
]
Please send over test-results.zip
Logs and images in ./test-results/
```
@gvwilson gvwilson added feature something new P2 needed for current cycle labels Nov 29, 2024
@gvwilson gvwilson requested a review from ayjayt November 29, 2024 14:56
@ayjayt ayjayt closed this Mar 25, 2025
@ayjayt
Copy link
Collaborator

ayjayt commented Mar 25, 2025

@gvwilson we changed strategy so I'm closing this but you still might use this as a checklist to remind yourself of what you expected?

@ayjayt ayjayt deleted the revisions-for-kaleido2 branch October 6, 2025 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature something new P2 needed for current cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants