Skip to content

Add pass through kopts dict for kaleido args #316

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

Merged
merged 5 commits into from
Apr 16, 2025
Merged

Add pass through kopts dict for kaleido args #316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ee484f
Add pass through kopts dict for kaleido args
ayjayt Apr 15, 2025
76d241e
Update CHANGELOG
ayjayt Apr 15, 2025
0c2d996
Remove separate n arg entirely
ayjayt Apr 16, 2025
2581bdb
Add NoneType check to dict destruct
ayjayt Apr 16, 2025
2dad799
Add kopts to calc_fig
ayjayt Apr 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/py/CHANGELOG.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
v1.0.0rc12
- Add `kopts` args to top-level shortcuts to pass args to `Kaleido(**kopts)`
v1.0.0rc11
- Write mocker tool to parameterize opts in tests
- Crop page to pdf size
Expand Down
32 changes: 21 additions & 11 deletions src/py/kaleido/__init__.py
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should add kopts argument to calc_fig as well for consistency

Copy link
Collaborator Author

@ayjayt ayjayt Apr 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

i want to move towards using BytesIO as a return for write_image instead of calc_fig which is sort of hackey, but added the kopts option anyway

Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,25 @@ async def calc_fig(
opts=None,
*,
topojson=None,
kopts=None,
):
"""
Return binary for plotly figure.

A convenience wrapper for `Kaleido.calc_fig()` which starts a `Kaleido` and
executes the `calc_fig()`.
It takes an additional argument, `kopts`, a dictionary of arguments to pass
to the kaleido process. See the `kaleido.Kaleido` docs. However,
`calc_fig()` will never use more than one processor, so any `n` value will
be overridden.


See documentation for `Kaleido.calc_fig()`.

"""
async with Kaleido(n=1) as k:
kopts = kopts or {}
kopts["n"] = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't override user-provided value for n

Suggested change
kopts["n"] = 1
if "n" not in kopts:
kopts["n"] = 1

Copy link
Collaborator Author

@ayjayt ayjayt Apr 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

calc_figs can't iterate, so it doesn't make sense for n>1 (its mentioned in the docs, that any value of n will be overwritten to 1)

async with Kaleido(**kopts) as k:
return await k.calc_fig(
fig,
path=path,
Expand All @@ -60,20 +68,21 @@ async def write_fig( # noqa: PLR0913 (too many args, complexity)
topojson=None,
error_log=None,
profiler=None,
n=1,
kopts=None,
):
"""
Write a plotly figure(s) to a file.

A convenience wrapper for `Kaleido.write_fig()` which starts a `Kaleido` and
executes the `write_fig()`.
It takes one additional argument, `n`, which can be used to set the number
of processes.
It takes an additional argument, `kopts`, a dictionary of arguments to pass
to the kaleido process. See the `kaleido.Kaleido` docs.


See documentation for `Kaleido.write_fig()`.
See documentation for `Kaleido.write_fig()` for the other arguments.

"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll need to do a kopts = kopts or {} here, right? Otherwise the dict operations will fail if kopts is None.

Copy link
Collaborator Author

@ayjayt ayjayt Apr 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

yeah

In [2]: foo(**None)
─────────────────────────────────────────
TypeError                                 Traceback (most recent call last)
Cell In[2], line 1
────▶ 1 foo(**None)

TypeError: __main__.foo() argument after ** must be a mapping, not NoneType

async with Kaleido(n=n) as k:
async with Kaleido(**(kopts or {})) as k:
await k.write_fig(
fig,
path=path,
Expand All @@ -89,20 +98,21 @@ async def write_fig_from_object(
*,
error_log=None,
profiler=None,
n=1,
kopts=None,
):
"""
Write a plotly figure(s) to a file.

A convenience wrapper for `Kaleido.write_fig_from_object()` which starts a
`Kaleido` and executes the `write_fig_from_object()`
It takes one additional argument, `n`, which can be used to set the number
of processes.
It takes an additional argument, `kopts`, a dictionary of arguments to pass
to the kaleido process. See the `kaleido.Kaleido` docs.

See documentation for `Kaleido.write_fig_from_object()`.
See documentation for `Kaleido.write_fig_from_object()` for the other
arguments.

"""
async with Kaleido(n=n) as k:
async with Kaleido(**(kopts or {})) as k:
await k.write_fig_from_object(
generator,
error_log=error_log,
Expand Down