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

docs: add an example demonstrating define_units() usage #446

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jrycw
Copy link
Collaborator

@jrycw jrycw commented Sep 17, 2024

Hello team,

As we know, define_units() powers GT.fmt_units() behind the scenes, but it seems to lack a clear example for users to quickly grasp its usage.

I’d like to propose an example that demonstrates how to use define_units() to render a string with unit annotations as a subtitle in the table header. This example highlights that define_units() can be used independently and within components that don’t yet support unit annotations (such as the table header).

Below is the final table for reference.
image

Copy link

codecov bot commented Sep 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.75%. Comparing base (ea03bbc) to head (2f19948).
Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #446   +/-   ##
=======================================
  Coverage   86.75%   86.75%           
=======================================
  Files          42       42           
  Lines        4702     4702           
=======================================
  Hits         4079     4079           
  Misses        623      623           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@machow machow requested a review from rich-iannone September 17, 2024 14:16
Copy link
Member

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

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

LGTM!

@rich-iannone
Copy link
Member

@machow this looks great to me. Please merge whenever convenient!

@jrycw jrycw changed the title Add an example demonstrating define_units() usage docs: add an example demonstrating define_units() usage Sep 18, 2024
@rich-iannone rich-iannone requested a review from machow September 18, 2024 16:09
@machow
Copy link
Collaborator

machow commented Sep 18, 2024

@rich-iannone afaict parsing the curly braces in inputs like "{{a_1}}" is not a job for define_units() (for example, we didn't list it out in the rules.

It seems like define_units() handling of curly braces may be a bug (and inconsistent)?:

from great_tables import define_units

# removes curly braces ----
define_units("{{a_1}}").to_html()
# > 'a<span style="white-space:nowrap;"><sub style="line-height:0;">1</sub></span>'

# Notice that the second piece in curly braces (b_2) still has the braces around it ----
define_units("{{a_1}} {{b_2}}").to_html()
# > 'a<span style="white-space:nowrap;"><sub style="line-height:0;">1}}</sub></span> {{b<span style="white-space:nowrap;"><sub style="line-height:0;">2</sub></span>'

What is the intended behavior of define_units? It seems like it should not parse braces, since that's afaict UnitStr.from_str() territory. So...

  • UnitStr.from_str() parses pieces out of braces,
  • then it passes those piece to define_units()

Does this sounds right, and this behavior of define_units() parsing 1 specific brace situation is a bug?

@machow machow self-assigned this Sep 18, 2024
@jrycw
Copy link
Collaborator Author

jrycw commented Sep 19, 2024

Thanks for pointing this out! I originally thought it was a cool feature and didn’t notice that.

@machow
Copy link
Collaborator

machow commented Sep 19, 2024

I definitely think there is a cool feature tucked away in .cols_label()! You can use {{}} as a shorthand for using define_units(). For example, cols_label(some_col="Some Col {{h[_0^3]}}")

But I wonder if we skipped an important piece when initially wiring up units. Based on this PR, it seems like exposing a function similar to html() and md() might help connect the dots?

Maybe something like this?:

  • explicit: cols_label(some_col = text("Some Col ", unit("h[_0^3]")))
  • shorthand: cols_label(some_col = "Some Col {{h[_0^3]}}")

This would provide a bridge into other structure things:

  • tab_header(title = text("Some title", unit("a_1"))

Could be nicer ways to handle though?! (Related to #439)

@rich-iannone rich-iannone assigned rich-iannone and unassigned machow Sep 30, 2024
@rich-iannone rich-iannone self-requested a review September 30, 2024 19:26
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.

3 participants