Skip to content

Commit

Permalink
Make st.help more resilient with conditional members (#8228)
Browse files Browse the repository at this point in the history
## Describe your changes

`st.help` leverages `dir(object)` to determine the members in the object
to expose to the docstring. If an object makes a member conditional
based on internal state, `st.help` can throw an `AttributeError`.
Ideally, in these cases, the developer of the object would implement the
`__dir__` method to handle conditional members, but Streamlit can be
more resilient in handling this situation.

The example that brought this up was in sklearn, and I created
scikit-learn/scikit-learn#28558 with them to
better handle this scenario.

## Testing Plan

- Unit Tests should cover the change with the member available and
unavailable

---

**Contribution License Agreement**

By submitting this pull request you agree that all contributions to this
project are made under the Apache 2.0 license.
  • Loading branch information
kmcgrady committed Mar 1, 2024
1 parent 6e70fdb commit 33680ba
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 20 deletions.
45 changes: 25 additions & 20 deletions lib/streamlit/elements/doc_string.py
Expand Up @@ -514,30 +514,35 @@ def _get_members(obj):
if attr_name.startswith("_"):
continue

is_computed_value = _is_computed_property(obj, attr_name)
try:
is_computed_value = _is_computed_property(obj, attr_name)
if is_computed_value:
parent_attr = getattr(obj.__class__, attr_name)

if is_computed_value:
parent_attr = getattr(obj.__class__, attr_name)
member_type = "property"

member_type = "property"

weight = 0
member_docs = _get_docstring(parent_attr)
member_value = None
else:
attr_value = getattr(obj, attr_name)
weight = _get_weight(attr_value)

human_readable_value = _get_human_readable_value(attr_value)

member_type = _get_type_as_str(attr_value)

if human_readable_value is None:
member_docs = _get_docstring(attr_value)
weight = 0
member_docs = _get_docstring(parent_attr)
member_value = None
else:
member_docs = None
member_value = human_readable_value
attr_value = getattr(obj, attr_name)
weight = _get_weight(attr_value)

human_readable_value = _get_human_readable_value(attr_value)

member_type = _get_type_as_str(attr_value)

if human_readable_value is None:
member_docs = _get_docstring(attr_value)
member_value = None
else:
member_docs = None
member_value = human_readable_value
except AttributeError:
# If there's an AttributeError, we can just skip it.
# This can happen when members are exposed with `dir()`
# but are conditionally unavailable.
continue

if member_type == "module":
# Don't pollute the output with all imported modules.
Expand Down
41 changes: 41 additions & 0 deletions lib/tests/streamlit/elements/doc_string_test.py
Expand Up @@ -33,6 +33,21 @@ def patch_varname_getter():
)


class ConditionalHello:
def __init__(self, available, ExceptionType=AttributeError):
self.available = available
self.ExceptionType = ExceptionType

def __getattribute__(self, name):
if name == "say_hello" and not self.available:
raise self.ExceptionType(f"{name} is not accessible when x is even")
else:
return object.__getattribute__(self, name)

def say_hello(self):
pass


class StHelpAPITest(DeltaGeneratorTestCase):
"""Test Public Streamlit Public APIs."""

Expand All @@ -48,3 +63,29 @@ def test_st_help(self):
el.doc_string.startswith("Change the current working directory")
)
self.assertEqual(f"posix.chdir(path)", el.value)

def test_st_help_with_available_conditional_members(self):
"""Test st.help with conditional members available"""

st.help(ConditionalHello(True))
el = self.get_delta_from_queue().new_element.doc_string
self.assertEqual("ConditionalHello", el.type)
member_names = [member.name for member in el.members]
self.assertIn("say_hello", member_names)

def test_st_help_with_unavailable_conditional_members(self):
"""Test st.help with conditional members not available
via AttributeError"""

st.help(ConditionalHello(False))
el = self.get_delta_from_queue().new_element.doc_string
self.assertEqual("ConditionalHello", el.type)
member_names = [member.name for member in el.members]
self.assertNotIn("say_hello", member_names)

def test_st_help_with_erroneous_members(self):
"""Test st.help with conditional members not available
via some non-AttributeError exception"""

with self.assertRaises(ValueError):
st.help(ConditionalHello(False, ValueError))

0 comments on commit 33680ba

Please sign in to comment.