Add color support to charts, and refactor things a bit. #7022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -305,14 +303,7 @@ def generate_chart(chart_type, data, width: int = 0, height: int = 0): | |||
| data = {"": []} | |||
|
|
|||
| if isinstance(data, pa.Table): | |||
| raise errors.StreamlitAPIException( | |||
There was a problem hiding this comment.
Moved this to its own error type. See bottom of the new code.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
sfc-gh-tteixeira
force-pushed
the
chart_colors
branch
3 times, most recently
from
616e778
to
299d047
Compare
sfc-gh-tteixeira
force-pushed
the
chart_colors
branch
from
299d047
to
de37f07
Compare
lib/streamlit/type_util.py
Outdated
| @@ -558,16 +597,16 @@ def convert_anything_to_df( | |||
|
|
|||
|
|
|||
| @overload | |||
| def ensure_iterable(obj: Iterable[V_co]) -> Iterable[V_co]: | |||
There was a problem hiding this comment.
This is just me fixing some pre-existing MyPy typing problems that were blocking this PR from passing tests.
…ting the orig DF.
…ill land in another PR.
| else: | ||
| return _melt_data(df=data, last_index=last_index) | ||
| out_data, *_ = prep_data(df, **add_rows_metadata.columns) |
There was a problem hiding this comment.
Properly handle the data in add_rows() by calling here the exact same prep_data step we use in _generate_chart()
| MELTED_Y_COLUMN_NAME = MELTED_Y_COLUMN_TITLE + PROTECTION_SUFFIX | ||
| MELTED_COLOR_COLUMN_NAME = MELTED_COLOR_COLUMN_TITLE + PROTECTION_SUFFIX | ||
|
|
||
| # Name we use for a column we know doesn't exist in the data, to address a Vega-Lite rendering bug |
There was a problem hiding this comment.
is there a link to this vega-lite bug?
|
This PR is pretty long so will review again later tonight or tomorrow morning. Also Cypress test (4) is failing and I tried rerunning it. Can you fix that? |
willhuang1997
approved these changes
Jul 27, 2023
|
PR ready! I just sent it out for QA. Waiting for results before I can merge. |
… since JSON stringifies keys anyways, but this is cleaner.
Merged
eric-skydio
pushed a commit
to eric-skydio/streamlit
that referenced
this pull request
Dec 20, 2023
* Add color support to charts, and refactor things a bit. * Fix tests and types. * Fix types in type_util * Removin unnecessary DF cloning, and add test to ensure we're not mutating the orig DF. * Fix tests in latest Pandas and Arrow. * Revert st_arrow_add_rows test to avoid too many changes in this PR. Will land in another PR. * Remove snaps that should be regenerated. * Don't show labels when no x or y specified (to match existing behavior). * Update snapshots (need to regenerate) * Fix bug where x axis was sometimes quantitative rather than temporal. * Fix x enc when no x_column. * Fix tests. * Revert to melting data by hand since Snowflake doesn't include Altair 4.2 * Improve error messages. * Fix MyPy types * Update snapshots * Fix empty check * Revert setup.py * Remove dead code. * simple rename * Remove more dead code * Address comments. * Fix snapshots * Fix stupid copy/paste bug >_< * Small cleanup. Always use str() around dataset ids. Should be a no-op since JSON stringifies keys anyways, but this is cleaner.
zyxue
pushed a commit
to zyxue/streamlit
that referenced
this pull request
Mar 22, 2024
* Add color support to charts, and refactor things a bit. * Fix tests and types. * Fix types in type_util * Removin unnecessary DF cloning, and add test to ensure we're not mutating the orig DF. * Fix tests in latest Pandas and Arrow. * Revert st_arrow_add_rows test to avoid too many changes in this PR. Will land in another PR. * Remove snaps that should be regenerated. * Don't show labels when no x or y specified (to match existing behavior). * Update snapshots (need to regenerate) * Fix bug where x axis was sometimes quantitative rather than temporal. * Fix x enc when no x_column. * Fix tests. * Revert to melting data by hand since Snowflake doesn't include Altair 4.2 * Improve error messages. * Fix MyPy types * Update snapshots * Fix empty check * Revert setup.py * Remove dead code. * simple rename * Remove more dead code * Address comments. * Fix snapshots * Fix stupid copy/paste bug >_< * Small cleanup. Always use str() around dataset ids. Should be a no-op since JSON stringifies keys anyways, but this is cleaner.
zyxue
pushed a commit
to zyxue/streamlit
that referenced
this pull request
Apr 16, 2024
* Add color support to charts, and refactor things a bit. * Fix tests and types. * Fix types in type_util * Removin unnecessary DF cloning, and add test to ensure we're not mutating the orig DF. * Fix tests in latest Pandas and Arrow. * Revert st_arrow_add_rows test to avoid too many changes in this PR. Will land in another PR. * Remove snaps that should be regenerated. * Don't show labels when no x or y specified (to match existing behavior). * Update snapshots (need to regenerate) * Fix bug where x axis was sometimes quantitative rather than temporal. * Fix x enc when no x_column. * Fix tests. * Revert to melting data by hand since Snowflake doesn't include Altair 4.2 * Improve error messages. * Fix MyPy types * Update snapshots * Fix empty check * Revert setup.py * Remove dead code. * simple rename * Remove more dead code * Address comments. * Fix snapshots * Fix stupid copy/paste bug >_< * Small cleanup. Always use str() around dataset ids. Should be a no-op since JSON stringifies keys anyways, but this is cleaner.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Add support for
colorargument in charts. As part of this work, I also refactored how charts work a bit, in order to prepare the land forst.scatter_chart, which is coming in a subsequent PR.This is PR 3 in the work to:
(PR 1 was a no-op that added the color_util.py library, which is used in this and the next PRs)
What else this does:
_generate_chart(), and next is_maybe_melt(). The good news is the new code is much more readable 😉titleattributes._generate_chart. This is just the right thing, and may even address some undiscovered bugs.How to review this
The main file you need to understand is
lib/streamlit/elements/arrow_altair.py, in particular the_generate_chart()function. It's a bit of a behemoth. Luckily, everything else (especially outside that file) is just supporting code that should be quite straight-forward to understand once you grok that function.GitHub Issue Link (if applicable)
n/a
Testing Plan
N/A - Explanation of why no additional tests are needed
✅ Unit Tests (JS and/or Python)
✅ E2E Tests
Any manual testing needed?
Yes. I'll request this from the QA team soon.
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.