-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat: add fmt_icon()
method
#515
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #515 +/- ##
==========================================
- Coverage 89.79% 89.79% -0.01%
==========================================
Files 45 45
Lines 5382 5431 +49
==========================================
+ Hits 4833 4877 +44
- Misses 549 554 +5 ☔ View full report in Codecov by Sentry. |
elif isinstance(self.stroke_width, (int, float)): | ||
stroke_width = f"{str(self.stroke_width)}px" | ||
else: | ||
stroke_width = self.stroke_width |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please test this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now tested.
if icon in self.fill_color: | ||
fill_color = self.fill_color[icon] | ||
else: | ||
fill_color = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please test this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests now present for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, if we can polars data wrangling in the second example
great_tables/_formats.py
Outdated
towny_mini = ( | ||
pl.from_pandas(towny) | ||
.select(["name", "csd_type", "population_2021"]) | ||
.filter(pl.col("csd_type").is_in(["city", "town"])) | ||
.group_by("csd_type", maintain_order=True) | ||
.agg([ | ||
pl.col("name").sort_by("population_2021", descending=True).head(5), | ||
pl.col("population_2021").sort(descending=True).head(5) | ||
]) | ||
.sort("csd_type") | ||
.explode(["name", "population_2021"]) | ||
.with_columns( | ||
csd_type = pl.when(pl.col("csd_type") == "town") | ||
.then(pl.lit("house-chimney")) | ||
.otherwise(pl.lit("city")) | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to just select a few rows of this data. E.g. towny[[0, 3, 5], :]
. Say maybe the top 2 in each of your groups. It seems easier to get into an example with less setup (a doesn't seem critical to have a comprehensive table).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took your approach and the setup for the example is far simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
This PR adds the
fmt_icon()
method. It depends on Font Awesome icons available in thefaicons
library, so that is a new dependency. Whenever that package is updated, new icons from the FA free set will be available. Here's an example of thefmt_icon()
method in use: