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

Works with Panel (sort of) #33

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MarcSkovMadsen
Copy link

@MarcSkovMadsen MarcSkovMadsen commented Aug 3, 2024

I would like to document that Quak works with Panel. Might also test the widget in alternative ways that could provide input for further improvements.

I see some issues though that I could not figure out where where coming from or how to solve.

Max-Height of quak widget set to 274px

I can't figure out where this is coming from or change it. Setting widget.layout.height="100%", widget.layout.height="600px" or widget.layout.max_height="100%" does not seem to change anything.

image

Lots of errors in terminal while loading and interacting with the widget

Traceback (most recent call last):
  File "/home/jovyan/repos/private/quak/.venv/lib/python3.11/site-packages/quak/_widget.py", line 64, in _handle_custom_msg
    result = self._conn.query(sql).arrow()
             ^^^^^^^^^^^^^^^^^^^^^
duckdb.duckdb.ParserException: Parser Error: syntax error at or near "50000000000"
2024-08-03 18:19:22,213 Error processing query
Traceback (most recent call last):
  File "/home/jovyan/repos/private/quak/.venv/lib/python3.11/site-packages/quak/_widget.py", line 68, in _handle_custom_msg
    self._conn.execute(sql)
duckdb.duckdb.BinderException: Binder Error: column "bronze" must appear in the GROUP BY clause or must be part of an aggregate function.
Either add it to the GROUP BY list, or use "ANY_VALUE(bronze)" if the exact value of "bronze" is not important.
LINE 4: ... TABLE IF NOT EXISTS cube_index_5407d6bc AS WITH "counts" AS (SELECT CASE
                                ...
                                                  ^
2024-08-03 18:19:22,332 Error processing query
Traceback (most recent call last):
  File "/home/jovyan/repos/private/quak/.venv/lib/python3.11/site-packages/quak/_widget.py", line 68, in _handle_custom_msg
    self._conn.execute(sql)
duckdb.duckdb.BinderException: Binder Error: column "bronze" must appear in the GROUP BY clause or must be part of an aggregate function.
Either add it to the GROUP BY list, or use "ANY_VALUE(bronze)" if the exact value of "bronze" is not important.
LINE 4: ... TABLE IF NOT EXISTS cube_index_3d676da9 AS WITH "counts" AS (SELECT CASE
                                ...
                                                  ^
2024-08-03 18:19:22,448 Error processing query
Traceback (most recent call last):
  File "/home/jovyan/repos/private/quak/.venv/lib/python3.11/site-packages/quak/_widget.py", line 68, in _handle_custom_msg
    self._conn.execute(sql)
duckdb.duckdb.BinderException: Binder Error: column "bronze" must appear in the GROUP BY clause or must be part of an aggregate function.
Either add it to the GROUP BY list, or use "ANY_VALUE(bronze)" if the exact value of "bronze" is not important.
LINE 4: ... TABLE IF NOT EXISTS cube_index_cc74a395 AS WITH "counts" AS (SELECT CASE

Dark Theme?

I would be nice with a Dark Theme well. Panel supports dark themes really work and I mostly use them in my work.

Panel Native Widget?

With Panel 1.5.0 (not yet released as of 2024-08-03) you also build AnyWidgetComponents with Panel. I.e. reuse the javascript and build the Python model using Python.

I think this repository and widget could be a test of AnyWidget as a standard supported by libraries outside Jupyter like Marimo and Panel. So if you are interested I could contribute a Panel version of the quak Widget. Let me know if you are interested. Thx.

@MarcSkovMadsen MarcSkovMadsen changed the title Works with Panel Works with Panel (sort of) Aug 3, 2024
@manzt
Copy link
Owner

manzt commented Aug 3, 2024

Max-Height of quak widget set to 274px

Yes, the height is currently hardcoded to show ~10 rows. We can add an API for this.

Lots of errors in terminal while loading and interacting with the widget

Duplicate of #21

Dark Theme?

Theming is a bag of worms. Right now I don't have the capacity to take this on as I'm wrapping up my PhD. It would be an API that I'd be open to and willing to support through CSS variables in the future.

Panel Native Widget?

I don't have the capacity to take this on and encourage someone from the Panel community who is familiar to take it on. When #16 is complete that should be much more straight forward.

Copy link
Owner

@manzt manzt left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! This section takes up almost a third of the README, which should primarily introduce quak. There isn't much here specific to quak, and to me this reads more like a tutorial and endorsement of Panel.

While Panel is great, anywidget works in many environments. To avoid showing preference, I intentionally choose a neutral tone. I emphasize that quak is an anywidget, and places where anywidgets work well should come to mind to readers.

In fact, I see it as an opportunity for frameworks to better emphasize their compatibility with anywidget if they want to be known for that support! I'd be happy to include a concise "works with" section and a link to a Panel tutorial instead.

@MarcSkovMadsen
Copy link
Author

MarcSkovMadsen commented Aug 4, 2024

Thanks for your feedback. Makes sense.

I've tried to minimize and make neutral. Let me know what you think. Thanks.

For context my contribution was inspired by what I have contributed in other places like https://github.com/whitphx/transformers.js.py#panel and https://developmentseed.org/lonboard/latest/ecosystem/panel/.

I believe the longer Transformers and lonboard tutorials are useful as they more specifically helps the imagination of the users to understand what is possible and how to do it. One way to do it here would be to add it via a detail/ summary html element. It would take up an empty line + the detail/ summary line in the README. I experimented with it, but settled on the current contribution as I believe its in line with your feedback.

@MarcSkovMadsen
Copy link
Author

I persisted the original tutorial here https://discourse.holoviz.org/t/works-with-quak/7537.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants