[pyOpenSci review] use `hasattr(x, "to_html")` instead of checking for a `UnitStr` instance
rich-iannone opened this issue · 1 comments
rich-iannone commented
From the pyOpenSci comment:
suggestion (_text.py): I wonder if we could use hasattr(x, "to_html") instead of checking for a UnitStr instance. That way we would 1. get rid of the circular import and 2. enable other types. I totally understand if you want to keep full control of what is rendered, though.
rich-iannone commented
At the time of review this was:
def _process_text(x: str | Text | None) -> str:
from great_tables._helpers import UnitStr
if x is None:
return ""
if isinstance(x, Md):
return _md_html(x.text)
elif isinstance(x, Html):
return x.text
elif isinstance(x, str):
return _html_escape(x)
elif isinstance(x, Text):
return x.text
elif isinstance(x, UnitStr):
return x.to_html()
else:
raise TypeError(f"Invalid type: {type(x)}")Now, we've moved to using a BaseText class and this greatly simplifies the HTML rendering:
great-tables/great_tables/_text.py
Line 71 in b32b55a
def _process_text(x: str | BaseText | None, context: str = "html") -> str:
if x is None:
return ""
escape_fn = _html_escape if context == "html" else _latex_escape
if isinstance(x, str):
return escape_fn(x)
elif isinstance(x, BaseText):
return x.to_html() if context == "html" else x.to_latex()
raise TypeError(f"Invalid type: {type(x)}")This work removes the circular import and enables other types.