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

Extend our st.cache MagicMock handling logic to Mock #2846

Merged
4 commits merged into from Feb 26, 2021
Merged

Extend our st.cache MagicMock handling logic to Mock #2846

4 commits merged into from Feb 26, 2021

Conversation

ghost
Copy link

@ghost ghost commented Feb 23, 2021

Fixes #2813.

In st.cache(), we currently don't hash MagicMock objects because they can cause an infinite recursion. Mock objects suffer the same problem, so this PR extends the logic to cover those as well.

@ghost ghost self-requested a review February 23, 2021 19:55
Copy link
Contributor

@tconkling tconkling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just needs a test.

Can we do isinstance(obj, unittest.mock.Mock) instead of testing for the exact class names? (MagicMock is a subclass of Mock, so this test would catch both of those.)

I don't think we actually need to test for mock.mock.Mock anymore; now that we're on Python 3, Mock/MagicMock is guaranteed to be in the standard library, which means we can also import Mock safely in this file without testing against the classes' name strings.

@ghost ghost merged commit a1a31fa into streamlit:develop Feb 26, 2021
@ghost ghost deleted the fix-stcache-stack-overflow branch February 26, 2021 01:27
sthagen added a commit to sthagen/streamlit-streamlit that referenced this pull request Feb 26, 2021
Extend our st.cache MagicMock handling logic to Mock (streamlit#2846)
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)
  ...
This pull request was closed.
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.

Testing a method with caching
1 participant