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

Fix issues #2 and #3, so the project works again on latest streamlits #4

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

Conversation

wyattscarpenter
Copy link

@wyattscarpenter wyattscarpenter commented Jun 19, 2024

Closes #2, #3.

(update: since these are blocking bugs, meaning the main project cannot be used at all on modern versions of streamlit and python, you can find my fork of this library here, and use it in your projects instead https://pypi.org/project/wfork-streamlit-profiler/)

I basically did the minimal thing that would work.

It seems to me that the "styling stuff" you were worried about still works fine, if that just means the profiler panel looks the same. (Tested with Python 3.10.12)

image

Let me know if there's anything else I need to investigate wrt this PR.

@hendriklohse
Copy link

Thanks! Would love to see this PR published on pypi @jrieke

@nicomunting
Copy link

I was running into the same issue and I can confirm that it now works as expected with this PR (Python 3.12.4 and Streamlit 1.36). I also verified that the changed import works on older versions of Streamlit (Python 3.9.19 and Streamlit 1.0). Apparently, it was never meant to be used directly as st.components.v1.html (see streamlit/streamlit#8644).

@wyattscarpenter
Copy link
Author

@hendriklohse in case it's helpful to you or others in your situation I should note that if your system has git then you can depend directly upon my git repo for my fixed version of this package. Make sure to use the patch-1 branch instead of main! Probably also leave a comment by it explaining this, because if this PR gets incorporated I'll probably remove my git repo.

I'm also considering publishing my fixed version as a package on pypi, although the git solution works so well it's hard to say why I would do so.

@wyattscarpenter
Copy link
Author

I realized I could save a minute on one of my deployments by doing this, actually, so my fix is now available at https://pypi.org/project/wfork-streamlit-profiler/

@wyattscarpenter
Copy link
Author

wyattscarpenter commented Aug 30, 2024

On reflection, I think this commit might in fact "break some of the styling stuff", because I think there's supposed to be a dark mode for pyinstrument, or at least the readme for current pyinstrument has a darkly-styled html display https://github.com/joerick/pyinstrument , and I've only ever gotten the light style of pyinstrument to display.

@hendriklohse
Copy link

hendriklohse commented Sep 9, 2024

Thanks! There were some security/access reasons for my project pip install was better than using the source but it seems to be implemented now, thanks a lot for this.

@nicomunting
Copy link

On reflection, I think this commit might in fact "break some of the styling stuff", because I think there's supposed to be a dark mode for pyinstrument, or at least the readme for current pyinstrument has a darkly-styled html display https://github.com/joerick/pyinstrument , and I've only ever gotten the light style of pyinstrument to display.

I checked this, but it appears dark mode of pyinstrument was never working with streamlit-profiler. I checked with streamlit-profiler 0.2.4 with py-instrument 4.1.1 on Python 3.9.21 both with Streamlit 1.0.0 and Streamlit 1.41.1.

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.

pyinstrument 4.1.1 / python 3.11 & 3.12 compatibility
3 participants