Skip to content

Better method signatures#88

Merged
tvst merged 47 commits intostreamlit:developfrom
tvst:better-method-signatures
Sep 19, 2019
Merged

Better method signatures#88
tvst merged 47 commits intostreamlit:developfrom
tvst:better-method-signatures

Conversation

@tvst
Copy link
Copy Markdown
Contributor

@tvst tvst commented Sep 11, 2019

Most DeltaGenerator methods are decorated with helper functions that insert special arguments into them. For example, the arugment element is inserted into almost every DeltaGenerator method, and ui_value is inserted into every widget method.

These arguments aren't be user-visible, in the sense that users can't pass their own element proto into these methods. So a while back we implemented the _wraps_with_cleaned_sig function to modify some special metadata in those methods, so the element argument wouldn't appear in the output of utilities like Python's help() function, ipython's ?, Streamlit's st.help, and our autogenerated API docs.

But we never updated the _wraps_with_cleaned_sig decorator to appropriately the user-hidden ui_value argument from widget methods. So that's what I'm doing in this PR.

In this PR, the actual fix was the introduction of the num_args_to_remove argument to the _wraps_with_cleaned_sig function. Everything else is just some cleanup and minor refactor, which should have no effect on actual logic.

tvst added 30 commits August 25, 2019 22:26
@tvst tvst requested review from jrhone and kantuni September 11, 2019 05:51
@tvst tvst self-assigned this Sep 11, 2019
@_wraps_with_cleaned_sig(method)
@_wraps_with_cleaned_sig(method, 1) # Remove self from sig.
def wrapped_method(self, *args, **kwargs):
return method(self, None, *args, **kwargs)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure I understand why we removed None here. It's because _wraps_with_cleaned_sig was always removing the first two args before but now we can tell it to remove just one?

Copy link
Copy Markdown
Contributor Author

@tvst tvst Sep 18, 2019

Choose a reason for hiding this comment

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

The only method that used this decorator was st.dataframe, and it had this signature:

def datafame(self, element, data, etc, etc):

...where element was actually not used in that method.

So when we called method(self, None, *args, **kargs) here, we were actually calling:

dg.dataframe(self=self, element=None, *args, **kwargs)

In this PR I removed the unused element argument, so we no longer need to pass None here.

@tvst tvst merged commit b673427 into streamlit:develop Sep 19, 2019
@tvst tvst deleted the better-method-signatures branch September 19, 2019 00:21
tconkling added a commit to tconkling/streamlit that referenced this pull request Sep 23, 2019
* develop:
  Release 0.46.0 (streamlit#170)
  Magic fixes (streamlit#138)
  [docs] Add analytics; redirect /secret/docs; fix compilation problem (streamlit#149)
  Fix bug for startup under windows (streamlit#144)
  Responsive layout (streamlit#104)
  Add basic PR template
  Better method signatures (streamlit#88)
  Publish docs to both /docs and /secret/docs, until we deprecate /secret/docs in January. (streamlit#141)
  Rename/report2app (streamlit#126)
tconkling added a commit to tconkling/streamlit that referenced this pull request Sep 23, 2019
* develop: (54 commits)
  Removing uber demo from streamlit repo (streamlit#159)
  Release 0.46.0 (streamlit#170)
  Magic fixes (streamlit#138)
  [docs] Add analytics; redirect /secret/docs; fix compilation problem (streamlit#149)
  Fix bug for startup under windows (streamlit#144)
  Responsive layout (streamlit#104)
  Add basic PR template
  Better method signatures (streamlit#88)
  Publish docs to both /docs and /secret/docs, until we deprecate /secret/docs in January. (streamlit#141)
  Rename/report2app (streamlit#126)
  Test running streamlit commands "bare" (streamlit#133)
  Updates to LP and sidebar. (streamlit#134)
  tests for z-index of date input popover in sidebar (streamlit#131)
  cypress test for escaping html in markdown (streamlit#125)
  On a Mac, check if xcode installed before requiring watchdog (streamlit#91)
  [Docs] Fix st.slider API in tutorial (streamlit#98)
  Sidebar exceptions (streamlit#107)
  Fixing unbound local variable (streamlit#110)
  Support hashing dataframes with unhashable objects. Gracefully f… (streamlit#118)
  Fix hashing if the object has a name but the name is not a string. (streamlit#117)
  ...
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.

2 participants