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

feat: start work to name the new-look package 'kaleido2' #232

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gvwilson
Copy link
Collaborator

@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
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.

1 participant