watch_file utility function
#2851
Merged
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
Merged
vdonato
reviewed
Feb 24, 2021
| return False | ||
|
|
||
| watcher_class(path, on_file_changed) | ||
| return True |
There was a problem hiding this comment.
Hm, I don't have a strong understanding of how the Python garbage collector works (so this may be a longshot), but could this cause any weirdness with the gc when using PollingFileWatcher?
The refcount on EventBasedFileWatchers should always be >0 since there's an internal singleton that keeps track of all the individual watchers, but in the case of PollingFileWatcher, we seem to lose the reference to it, which would make it a candidate to be garbage collected and the on_file_changed callback mysteriously stop working.
Maybe returning a ref to an Optional[FileWatcherType] could be done here, but I'm not entirely sure if my concerns are valid to begin with.
There was a problem hiding this comment.
Whoops, never mind I realized what I was missing. The ThreadPoolExecutor in PollingFileWatcher has a ref to the instance (which, now that I'm thinking about it, should have been obvious since it needs a ref to the callback to run it)
There was a problem hiding this comment.
I added a comment to PollingFileWatcher to clarify that it's not necessary to retain an instance to have the polling work, because it's definitely non-obvious at first glance!
* develop: Align st.beta_columns (streamlit#2811)
vdonato
approved these changes
Feb 24, 2021
| return False | ||
|
|
||
| watcher_class(path, on_file_changed) | ||
| return True |
There was a problem hiding this comment.
Whoops, never mind I realized what I was missing. The ThreadPoolExecutor in PollingFileWatcher has a ref to the instance (which, now that I'm thinking about it, should have been obvious since it needs a ref to the callback to run it)
tconkling
added a commit
to tconkling/streamlit
that referenced
this pull request
Feb 24, 2021
* develop: `watch_file` utility function (streamlit#2851) Align st.beta_columns (streamlit#2811)
vdonato
added a commit
that referenced
this pull request
Feb 24, 2021
* `watch_file` utility function (#2851) Adds two file-watching utility functions: - `get_file_watcher_class()` (returns the correct class to use for watching files, based on the user's config. This has been hoisted out of `local_sources_watcher`). - `watch_file()` (watches a single file using the correct fileWatcher) This de-couples file-watching from `LocalSourcesWatcher` so that we can easily use it outside of that context. (Secrets.toml and config.toml reloading will use this code.) * st.beta_secrets (#2757) Adds the `st.secrets` module to streamlit (as `st.beta_secrets`). - `./.streamlit/secrets.toml` is loaded on startup if it exists, and is accessible via `st.beta_secrets` - If secrets.toml is modified, it will be automatically reloaded - Top-level key-value pairs in secrets.toml are also added to `os.environ` * hide empty columns on mobile (#2756) Co-authored-by: Tim Conkling <tconkling@gmail.com> Co-authored-by: bh-streamlit <76131205+bh-streamlit@users.noreply.github.com>
tconkling
added a commit
to tconkling/streamlit
that referenced
this pull request
Feb 24, 2021
* develop: st.beta_secrets (streamlit#2757) `watch_file` utility function (streamlit#2851) Align st.beta_columns (streamlit#2811)
tconkling
added a commit
that referenced
this pull request
Mar 1, 2021
* develop: (29 commits) Update bug_report.md Refactor CodeBlock.tsx (#2814) Remove copy button for empty codeblocks (#2808) Add image format deprecation config with expiration (#2865) Remove unneeded "use_column_width=True" from our doc examples (#2692) Extend our st.cache MagicMock handling logic to Mock (#2846) save work (#2862) Remove .stale-element class (#2848) Release 0.77 (#2849) Fix watchdog import failure (#2856) 🔥 Fully remove `format` param from st.image (#2637) Don't memoize config._server_headless (#2858) hide empty columns on mobile (#2756) st.beta_secrets (#2757) `watch_file` utility function (#2851) Align st.beta_columns (#2811) Update "showErrorDetails" config description and docs (#2841) Pause Dependabot updates for non-security-related issues (#2840) client.showTracebacks -> showErrorDetails (per product) (#2837) Color picker - show value (#2817) ...
ghost
pushed a commit
that referenced
this pull request
Mar 15, 2021
* Tooltips: add `help` keyword/field to protobufs and API (#2586) * add help keyword/field to protobufs and api * fix pylint stuff * fix tests * move help to last kwarg * Add tooltip to TextInput (#2588) * add help keyword/field to protobufs and api * add tooltip to TextInput * remove unnecessary code * fix pylint * fix pylint stuff * fix tests * remove unnecessary change * add e2e test * remove unnecessary change * see if this fixes some cypress tests * make requested changes * make other requested change * fix failing test * Add tooltips to even more widgets (#2594) * add help keyword/field to protobufs and api * add tooltip to TextInput * remove unnecessary code * fix pylint * fix pylint stuff * fix tests * remove unnecessary change * add e2e test * remove unnecessary change * save work * get tooltip working with button * remove unnecessary change * see if this fixes some cypress tests * make requested changes * fix merge * fix imports * update st_tooltips.spec.js * destructure * Add tooltips to more widgets (#2593) * add help keyword/field to protobufs and api * add tooltip to TextInput * remove unnecessary code * fix pylint * fix pylint stuff * fix tests * remove unnecessary change * add e2e test * remove unnecessary change * add tooltips for st.checkbox, st.radio, st.number_input * fix import order * see if this fixes some cypress tests * make requested changes * fix * fix test * add tests and fix tooltip positioning * Add tooltips to visual elements (#2614) * add tooltips to visual elements * fix newline * Add missing help keywords and protos (#2620) * add missing help keywords and protos * fix test * add missing function/proto * fix newline * Add tooltips to Markdown elements (#2621) * add tooltips to visual elements * fix newline * add missing help keywords and protos * add new tests * add remaining tooltips * add style change * move changes to a different branch * add newline * fix tooltips for a few inputs (#2631) * Theme selector (#2663) * Add selector + themes * Default theme + cleanup * Rename main theme to light theme (#2670) * Add customTheme config options (#2671) * Reorder imports in config.py to be alphabetical * Allow capital letters in config sections We don't currently have any two-word config sections but want to add a `customTheme` section and need it to be camelCase for consistency with how individual config options are named. * Add config options for custom themes * Have customTheme.font default to "sans serif" * Convert themes to config and add baseweb dark mode (#2675) * Convert themes to config and add baseweb dark mode * PR comments - rename + typo * Add CustomThemeConfig protos and pass theme config to the client (#2681) * Add CustomThemeConfig proto message type * Pull some NewReport msg construction into helper functions Also wrote a test. * Read customTheme config and include it in Initialize msg * Promote custom_theme to be a child of the Initialize proto * Add config.get_options_by_section * Use config.get_options_by_section to marshal custom_theme protos * Fix grammatical error in comment * Rename get_options_by_section -> get_options_for_section * Add types to new functions in report_session.py * Reorder imports in report_session.py * Set theme (#2687) * Set theme + reorg theme to use ThemeConfig * PR feedback * Add serif font (#2691) * Create custom theme (#2693) * Create custom theme * Tests + fixes from tests * Fix linting * Fix light + dark theme colors * Fix sidebar theme + styling * Fix report_session_test (#2707) Apparently a constructor signature changed on `develop`, causing a newly added test in `report_session_test` to fail. * save work * save work * add final change * Theming Color Hookups (#2710) * wip * Update styling variables * Remove started global styles * revert unwanted changes * Theming global styles (#2714) * Set up globalStyles * copy and start updating theme.scss -> emotion * copy and convert reboot.scss -> emotion * convert copied sass -> emotion theme * Update comment * add tooltips to rest of widgets * Apply theme to VegaLite Charts (#2708) * Apply theme to VegaLite Charts * Substitute theme.colors.bodyText -> textColor and update snapshots * save work * make tests pass * Have plotly charts apply theme options (#2719) * Destructure theme in VegaLiteChart.tsx * Have plotly charts apply theme options This isn't totally done yet as plotly has two background colors: the "paper" background color and "plot" background color, and the default plot background color looks fine in light mode but pretty silly in dark. We could fix this by changing plotly chart style to match our other plot types, but for now I just left a TODO and can circle back to this later. * rename config names (#2740) * Theming sidebar (#2718) * Remove sidebar theme + cleanup for sidebar * Unit tests * Checkbox * Fix sidebar gradiant * Update sidebar snapshot + radio button * VegaLite bg fix * Calendar header * Add snapshot * PR feedback * Opps * Theming widget colors (#2742) * Create widget variables based on provided colors + updates to widgets * Expander text + updated snapshot * Theming - available themes (#2744) * Add auto as available theme + fix constant adding of custom * Update local store with custom theme if active * Make sure auto gets latest auto, not local storage * Fix test * Do not save auto theme to storage * Theming widgets (#2745) * Colors for dropdowns + fullscreen buttons * Color in stuff for status widget * Remove unnecessary white color * Update tests * Fix typing and remove unnecessary typing. Thanks ken * Handle test setting Auto with previous saved value (#2747) * Set plotly chart background to theme secondaryBg color (#2748) * Set plotly chart background to theme secondaryBg color Now that the sidebar background color has been renamed/repurposed to be a secondaryBg color, it seems appropriate to use it for this. This'll probably need a product review down the line, but this now looks pretty reasonable in both light and dark mode compared to before. * Add snapshot test * Set RNG seed * Update snapshot * Theming OS change (#2755) * Add event listener for OS theme change * Doing too much reorg, only update auto theme if auto was last selected * Pass theme object to custom components (#2749) * Pass theme object to iframe Note that we still need to update the custom component template along with the code living in component-lib to use the theme object. * Pass theme to component and set css vars in component-lib * Don't set font-family in component-lib css for now * Change up visual of theme selector (#2759) * Set custom theme by default if one exists (#2758) * Add customTheme.setAsDefault config var and pass to frontend * Set custom theme by default if flag is true * Theming dark primary (#2763) * Change up visual of theme selector * Change dark mode primary color * Change default theme to create auto theme, not get system theme (#2768) * Make theme-related types in component-lib more strict (#2762) * Fixing monospace up (#2769) * Change default theme to create auto theme, not get system theme * Rename mono, fix mono, hardcode some things to not mono * tests * handle font = 0 * Bump component-lib version (#2772) * Theming - Fail gracefully for invalid colors (#2771) * Fail gracefully for hex values * fix merging test * Rename config options (#2775) * Export theme type from component-lib (#2777) * merge (#2785) * Validate customTheme config options (#2776) * Have customTheme.name default to "Custom Theme" * Validate customTheme config options This entails throwing an error when * The theme name specified is reserved ("Auto", "Dark", or "Light") * The theme config does not specify all required options * Appease mypy * Move theme validation into config.py This needs to happen since validation has to occur when a user calls `streamlit run <filepath>` rather than on script run. * Fix some name issues that came up after rebase * Address PR comments * Theming docstring (#2788) * Pick colors for docstring * Add E2E * Snapshot * Remove custom theme option when server doesn't send one (#2787) * Remove custom theme option when server doesn't send one * Don't ignore user preferences on theme remove * Move comment to right place * Remove unused imports * Black or white pre based on bg (#2794) * merge conflicts (#2797) * Theming settings (#2799) * Quick styling * Change radio to dropdown + styling + rename auto * Add themes to snapshot * Update active theme on custom theme selection (#2800) * Save on settings dialog changes and remove save/cancel buttons (#2802) * Change theme object passed to components to match options in config.py (#2806) * Have theme passed to components match options in config.py * Have component-lib use new names * Theme Creator UI (#2786) * Quick and dirty prototype * Adjust color picker UI + add show value option * Redesign theme creator + copy to clipboard * Fix tests + typo * Auto update * toComponentTheme -> toThemeInput * Add font * Tests * PR feedback * Add developer mode (#2835) * Fix font parsing + typo in config copy (#2842) * Uppercase color picker display (#2850) * Theming contrast (#2773) * Add func to ensure contrasting colors * Color updates * Color adjustments * Update snapshots * Update active theme when editing custom theme (#2845) * Change to provider + selectbox didupdate * PR comment * Promote Config and CustomTheme protos to be children of NewReport (#2843) These messages were originally children of `NewReport.Initialize`, but as we're making the change to have (some) config options auto-reload on file save, this shouldn't be the case anymore given the initialize message by definition only contains things that never change. * More font conversions (#2852) * Fix font conversions * PR comments * Check for any merge weirdness merging develop into theming (#2854) * `watch_file` utility function (#2851) Adds two file-watching utility functions: - `get_file_watcher_class()` (returns the correct class to use for watching files, based on the user's config. This has been hoisted out of `local_sources_watcher`). - `watch_file()` (watches a single file using the correct fileWatcher) This de-couples file-watching from `LocalSourcesWatcher` so that we can easily use it outside of that context. (Secrets.toml and config.toml reloading will use this code.) * st.beta_secrets (#2757) Adds the `st.secrets` module to streamlit (as `st.beta_secrets`). - `./.streamlit/secrets.toml` is loaded on startup if it exists, and is accessible via `st.beta_secrets` - If secrets.toml is modified, it will be automatically reloaded - Top-level key-value pairs in secrets.toml are also added to `os.environ` * hide empty columns on mobile (#2756) Co-authored-by: Tim Conkling <tconkling@gmail.com> Co-authored-by: bh-streamlit <76131205+bh-streamlit@users.noreply.github.com> * Theming creator v2 (#2853) * Update copy + edit active theme * Change on copied * get rid of props * Theming: Always pick the user choice upon reload (#2857) * Update copy + edit active theme * Change on copied * get rid of props * Always take users preference * Fix tests * Cleanup * Theming optional configs (#2859) * make secondary color optional * Check theme completeness * Theming creator copy (#2863) * make secondary color optional * Check theme completeness * Copy changes * If custom theme active, update in case it changed (#2864) * Make config.{get_option, set_option, get_options_for_section, parse_config_file} thread-safe (#2860) * Move variables/consts closer to where they're used in config.py * Make config.{get_option, set_option, get_options_for_section, parse_config_file} thread-safe This needs to be done since it'll soon be possible for config files to be reloaded on file change, so we'll need to ensure that threads trying to access/change config options don't try to do so when config.toml files are being re-parsed. * Double-check if work is needed after grabbing lock * Rework config file parsing to unset removed options (#2866) * Rework config file parsing to unset removed options `parse_config_file` (now named `get_config_options`) was previously written in a way where naively rerunning the function would fail to remove config options that were deleted from a file on initial load. This commit restructures things to create a `_config_options_template` when options are initially defined, then copy and update the template when loading config options from files / cli flags. * Add a missing unit test for get_config_options with flags * Add assert * Rerun scripts on config file change (#2869) * Rerun scripts on config file change There's still a bit of polish to do here. Most notably, we're currently just naively reloading the script whenever the config gets re-parsed, which is fine, but we want to additionally give the user a heads up when they change config options that require a full server restart. * Remove redundant call to localStorage.removeItem * Use secondary color more (#2882) * Use secondary color more * Snapshot update * Relax theme validation to just print warnings instead of raising (#2878) * Relax theme validation to just print warnings instead of raising Raising exceptions here now seems like a bad user experience since it's very likely that the user will get annoyed by things exploding when they're live-editing config files to see changes get applied. * Add more _validate_theme tests Also fixed some existing ones that no longer tested what we needed them to when we changed the function to warn instead of raise. * Do snapshots in both light + dark? (#2900) * Do snapshots in both light + dark? * Add snapshots * Move show_config to the new config_util.py (#2899) * Move show_config to the new config_util.py config.py is the longest .py file we have, and I was about to add more stuff to it, so I thought it was about time to try to split it up. It's actually pretty hard to move things out of this file given all its dependencies, but at the very least one of the longest functions was easily movable, and the new stuff I'm about to add can go into config_util. * Just grab lock instead of having a nearly paragraph-long comment * Design iteration on tooltips (#2915) * save work * change tooltipicon * fix js test * fix bugs * fix lint * Fix issues due to merge * fix type error * Warn if the user edits a [server] config option (#2901) * Drop the ConfigOptions type alias I've been thinking this alias wasn't really useful given how close it is to ConfigOption (singular), then I made a mistake because I misread it for the other :( * Warn if the user edits a [server] config option We can't always automatically apply these without a full server restart, so we should bug the user to reboot streamlit. Note that warning for all server config options is overly broad, but it's certainly better to over-warn users than under-warn them in this case. * Use parameterized in tests * Removed unused import * Don't overwrite config options set via flag on config.toml reload (#2898) * Don't overwrite config options set via flag on config.toml reload This was a known issue when we released Theming v0.4, but we decided that it was minor enough that fixing it could be pushed back until after that beta was released (presumably it should be rare for a user to set config options both via flag as well as config.toml file). * Add clarifying comment * Rework tests to work with both python 3.6 and 3.8 * Make names in dict comprehension more consistent * Remove the theme.name option (#2912) * Remove the theme.name option This is a task from the product launch review. Instead of letting developers name their themes, we'll just give any custom theme the name "Custom Theme" in the theme selector menu. * Hard code the string "Custom Theme" less Also rename AUTO_THEME -> AUTO_THEME_NAME * Wrap up design pass on theming (#2921) * style scrollbars * Fix header gradient color * Style menu and dialogs * Style everything else! * Fix small text in dialog * Make button border use fadedText10, like other borders. * Fix slider active color. * Improve "made with streamlit" text * Remove remaining usage of secondaryColor * Update enzyme snapshots for SettingsDialog and ThemeCreator * Get rid of StyledThemeColorPicker We can just use the ColorPicker directly now that we don't have any styles to override for it. * Fix (non-snapshot) js unit tests * Don't use primary color for background when no radio options * Don't use primary color for border when checkbox disabled * Update a TON of snapshots * Use fadedText colors more where equivalent * Move derived color computation into a helper and other misc cleanup * Use shorthand style props for borders where possible * Remove unnoticed reference to theme.name This one managed to sneak by since it has no effect on anything in the case where theme.name is never set. * Address PR comments * Get rid of an unnecessary import Co-authored-by: Thiago Teixeira <thiago@streamlit.io> * Make tooltips and theming play nicely together (#2939) The changes in this commit aren't used yet, but they will be very soon as tooltips are needed for the theme creator dialog. Aside from pulling in some code that lives in the tooltips feature branch a bit early, in this commit we also tweak tooltip colors so that they adjust along with the active theme. * Stop warn logs from being spammed every time a checkbox is rendered (#2938) This slipped by when the design pass PR was merged as I guess I never had a browser console open at the right time 😢 I happened to notice a ton of warn logs being spammed to the browser console whenever a checkbox was rendered. Unfortunately, this seems to be due to the incompatibility of the short/long-hand versions of these names and the fact that baseweb uses the long versions for some components, so we don't have much of a choice other than to be very, very verbose as well. * Theming markdown and json fixes (#2946) * Pick a reasonable ReactJson theme depending on background color * Brighten up markdown links a bit on dark backgrounds * Add a bunch of st_markdown snapshots * Don't reload theme on every app rerun (#2937) * Don't reload theme on every app rerun Currently, the theme reloads on app rerun when a user changes the value of a widget, which is because we're applying the theme in the config file without taking into account whether that theme has actually changed. This doesn't work well if a user was playing around with colors in the theme creator dialog, so we should instead only make custom-theme related changes if a theme config option was actually modified. * Use join instead of reduce and add a clarifying comment * Rework Theme Creator UI (#2940) * Rework Theme Creator UI * Moves the help text into tooltips * Has the theme creator be its own dialog instead of being an expandable part of the Settings dialog * Changes the layout of the Theme Creator to have two columns * Style all dialogs. * Update snapshots and fix other tests * Get rid of some now-empty styled components * Address PR feedback Co-authored-by: Thiago Teixeira <thiago@streamlit.io> * Tooltips + theming design changes (#2956) * save work * fix padding * use linkText * just remove link style Co-authored-by: karrie <karrie@streamlit.io> Co-authored-by: Vincent Donato <vincent@streamlit.io> Co-authored-by: Tim Conkling <tconkling@gmail.com> Co-authored-by: Thiago Teixeira <thiago@streamlit.io>
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.
Adds two file-watching utility functions:
get_file_watcher_class()(returns the correct class to use for watching files, based on the user's config. This has been hoisted out oflocal_sources_watcher).watch_file()(watches a single file using the correct fileWatcher)This de-couples file-watching from
LocalSourcesWatcherso that we can easily use it outside of that context. (Secrets.toml and config.toml reloading will use this code.)