[Horizontal Layout] Implement with DG#1964
[Horizontal Layout] Implement with DG#1964akrolsmir merged 17 commits intostreamlit:horiz-layoutfrom
with DG#1964Conversation
* try updating in virtualenv * new cache * am i using 39.0.1 and if so why is it failing? * somewhere somehow we are upgrading to setuptools 50.0.3... but where..? * looks like somewhere between setup + pipenv-install we get a newer version of setuptools * looks like somewhere between setup + pipenv-install we get a newer version of setuptools * force reinstall * looks like it's test requirements messing us up? * force reinstall + cleanup debugs
|
Would love any thoughts on how to scalably test that every st.element is supported using |
Actually, less concerned now that all "protected" methods" of DeltaGenerator start with self = self._active_dg. Incidentally, I'm open to the idea that "overwriting self is bad style; we should create a new variable instead". Slightly leaning towards overwriting self since it's fewer lines of code churn, though. |
karriebear
left a comment
karriebear
left a comment
There was a problem hiding this comment.
I think we need to take a look at containers in sidebar here but happy to talk through
|
|
||
| _main = _DeltaGenerator(container=_BlockPath_pb2.BlockPath.MAIN) | ||
| sidebar = _DeltaGenerator(container=_BlockPath_pb2.BlockPath.SIDEBAR) | ||
| sidebar = _DeltaGenerator(container=_BlockPath_pb2.BlockPath.SIDEBAR, parent=_main) |
There was a problem hiding this comment.
i think this is more a diagram update but based on the diagram, i would have expected sidebar and main to be 2 root trees
There was a problem hiding this comment.
Yeah, I tried to call this out specifically in the "With DG" area (lower right) but it's probably still confusing, apologies. The tree of DGs is a separate concept from the Element/Render tree:
- We want to do
with st.sidebarand be able to return control to the main DG afterwards, so it follows that sidebar must be a child of main DG in the DG Tree - Sidebar and Main have two separate render trees on the javascript side, so they're rooted separately on the Element Tree
Would love thoughts on how to make this concept clearer (and/or change the code implementation so they're merged???)
There was a problem hiding this comment.
or the more simpler thing is "Karrie actually read the diagram"
lib/streamlit/delta_generator.py
Outdated
|
|
||
| """ | ||
| # Switch to the active DeltaGenerator, in case we're in a `with` block. | ||
| self = self._active_dg |
There was a problem hiding this comment.
i dont think it hurts to have it here but i'm not quite sure how with is supposed to be used here? my natural assumption is a
with table:
st.add_rows()
but then we need to add add_rows to init?
if it's like below, then i don't think we need the active_dg?
my_table = st.table(df1)
with container:
my_table.add_rows(df2)
There was a problem hiding this comment.
Hm, that's a really good point that I didn't consider -- how add_rows works with with. I initially copied it into here with the guiding principle of "public methods of DeltaGenerator should all set self = self._active_dg", to try and head off any accidental cases where we forget to switch DGs.
But on inspection, add_rows isn't currently one of those; we can just stick with _enqueue, _get_coordinates, and any container-like elements, I think. I'll remove this, and try and think of a better rule for when we need to use active DG.
I'm with you, I like the overwriting 😳 mostly because it would look too messier otherwise |
Per Karrie's suggestion
* [Horizontal Layout] Implement st.container() (#1961) * Fix up dg._block() to be dg.container() * Add a unit test * Add an e2e test for st.container * Minor cleanups * Clean up variable names and comments * Clean up deltaPath code and docs * [Horizontal Layout] Implement `with DG` (#1964) * Force reinstall of setuptools to 49.6.0 (#1962) * try updating in virtualenv * new cache * am i using 39.0.1 and if so why is it failing? * somewhere somehow we are upgrading to setuptools 50.0.3... but where..? * looks like somewhere between setup + pipenv-install we get a newer version of setuptools * looks like somewhere between setup + pipenv-install we get a newer version of setuptools * force reinstall * looks like it's test requirements messing us up? * force reinstall + cleanup debugs * Fix up dg._block() to be dg.container() * Add a unit test * Add an e2e test for st.container * Minor cleanups * Clean up variable names and comments * Clean up deltaPath code and docs * Organize DGs in a tree, to track the `with DG` * Enable `with` in every st.foo call * Appease mypy with # type: ignore * More sacrifices at the altar of mypy * Add unit and e2e tests for with * Actually, don't rely on each DG mixin to call _active_dg Revert "Appease mypy with # type: ignore" This reverts commit 4ba7c47. Revert "Enable `with` in every st.foo call" This reverts commit 9b467d6. * Use the active DG for all "protected" class methods * Clean up DG docs * Don't switch DGs from add_rows Per Karrie's suggestion Co-authored-by: karrie <[email protected]> * [horiz-layout] Add collapsible_container (#1978) * Add collapsible_container * Add default collapsed * make label required + initial docs * Consolidate with WIP column work * Address PR comments for more cleanliness * Update e2e tests * Rename collapsible container (#1993) * rename to expander * Fix tests * Rename collapsible to expandable * [Horiz Layout] Columns (#1979) * Implement columns with variable widths * Add a e2e test for columns * Add a unit test * Update Prettier to 1.19 to allow optional chaining https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining * Fix e2e test * Downgrade Prettier back to 1.18 * Empty columns now take up space * Remove duplicate function from merge conflict * Use flex-gap instead of margin, so column sides don't get padded * Minor tweaks requested by code review * Prefix new layout methods with `beta_` Co-authored-by: karrie <[email protected]>
2 participants
See design at https://www.notion.so/streamlit/Eng-Design-d78f8786ff964795bd23545333e590e5