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

Make st.help more resilient with conditional members #8228

Merged
merged 4 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 25 additions & 20 deletions lib/streamlit/elements/doc_string.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,30 +514,35 @@
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:
Fixed Show fixed Hide fixed
# If there's any exception, 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
32 changes: 32 additions & 0 deletions lib/tests/streamlit/elements/doc_string_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ def patch_varname_getter():
)


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

def __getattribute__(self, name):
if name == "say_hello" and not self.available:
raise AttributeError(f"{name} is not accessible when x is even")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: the error message here seems unrelated to how we actually decide whether to throw an error in this method

else:
return object.__getattribute__(self, name)

def say_hello(self):
print("Hello")
Copy link
Collaborator

Choose a reason for hiding this comment

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

super nit: doesn't really matter since this method is never called, but it might be slightly preferable to have the body of this method be a no-op to not have a print statement in the code



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

Expand All @@ -48,3 +62,21 @@ 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.assertTrue("say_hello" in member_names)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit (here and below): I think there's an assertIn method that makes this more explicit


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

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.assertTrue("say_hello" not in member_names)