-
Notifications
You must be signed in to change notification settings - Fork 3k
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
remove unicode characters from demo script filenames #5067
Conversation
@giokara-oqton thanks for the PR 👍 I want others to tell they opinion on this, but I feel like this gives people more accessibility. @giokara-oqton oh, ok, I can see it's related to Bazel issue |
CC @jrieke as I believe this will come down to a product decision. If I'm remembering correctly, this issue is known, but work on fixing it wasn't prioritized as we believed the extent of the impact was relatively minor. |
Hey @giokara-oqton, thanks for this PR! I think we want to keep the emojis in the We do also want to find another way of adding emojis/icons to pages. But we first have to come up with a solid API to do this (and it's not our no. 1 priority right now), so it will probably take a few months. |
Hi @jrieke, not sure if it's clear from above, but this PR is not about removing emojis from the demo pages. The only change is removing the emojis from the filenames of the demo pages. Is this acceptable to you? Any changes on how emojis are supported within pages can be done separately I would expect. Bazel is the chosen build system within our organisation and so any streamlit application needs to be run through Bazel. Currently we have issues using streamlit because of the emojis (non-ASCII characters) in the filenames. |
Oh sorry for the delay. Streamlit reads the emojis from these filenames to set them in the multipage app. So if we remove them from the filenames, it will also remove them from the sidebar navigation in the |
Adding another +1 to this PR here. We are also currently stuck on streamlit 0.9 since bazel can't handle these file names at all. Is it possible to add the emoji as suggestion/comment inside the code instead? I know there are not that many bazel users but it does feel bad not able to use newer version due to example files :(. |
So the solution for those using Bazel and do not require newer functionality like emojis in the page, seems to be downgrading for now... For us, downgrading to 1.9.2 was sufficient (no emojis in the demo pages). |
This is blocking an upgrade to 1.11.1 for us as well. Considering that version patches a security vulnerability, we would really like to update, and being blocked by emojis in demo files indeed feels annoying. |
We are in exactly the same position - can't upgrade to fix the security issue due to bazel's poor unicode support. |
Hey all! We discussed this again since we got a lot of similar complaints. We decided that we'll merge this and remove the emojis from all filenames for now. Thanks to all here for surfacing your feedback! ❤️ |
Ok turns out we can't merge this right away because of a merge conflict 😉 So I'm closing this and we'll replicate the same changes in a separate PR, so that we can merge quickly and get this into the upcoming 1.12.0 release on Thursday. |
📚 Context
Please describe the project or issue background here
What kind of change does this PR introduce?
🧠 Description of Changes
Add bullet points summarizing your changes here
Revised:
Current:
🧪 Testing Done
🌐 References
Does this depend on other work, documents, or tickets?
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.