-
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
Add blog post demonstrating the usage of GT.fmt_image()
and vals.fmt_image()
#558
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #558 +/- ##
=======================================
Coverage 90.66% 90.67%
=======================================
Files 46 46
Lines 5367 5381 +14
=======================================
+ Hits 4866 4879 +13
- Misses 501 502 +1 ☔ View full report in Codecov by Sentry. |
Thanks for submitting this post! We'll read it over shortly. To address the question:
It's a feature (the great-tables/great_tables/_gt_data.py Line 357 in b32b55a
Most of the time, this works well. In the case of your example, those metro line numbers in And using a letter in place of any of the numbers will result in left alignment of the I think that there should probably be an |
@rich-iannone , thank you for the prompt response and for pointing me to the relevant source code. |
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.
Thanks for this PR! This post is incredible, and reads really nicely! The 4 body case studies seem great in terms of prep and example data.
There are two pieces in those case studies that I think could be tweaked a bit to motivate the "why" of the cases. This is important IMO because it tells them what they'll get out of each case study (especially 3 and 4, which are convenience arguments).
Here are the two pieces:
- The summary bullets at the beginning of the section (e.g. add text after to explain "why"; see comment)
- The explanations just before the DataFrame code in each case study.
- (I put an example in my case 1 comment)
- Since the code preparing tables is folded (a nice move!), you could remove the paragraphs above any folded code explaining what it does.
- Instead of the code explanations, a sentence prep'ing people on the table output might help orient them. What should they notice in each case study table?
I glanced through the other sections and they look so great!
docs/blog/rendering-images/index.qmd
Outdated
* **Case 1**: Local file paths. | ||
* **Case 2**: Full HTTP/HTTPS URLs. | ||
* **Case 3**: File names with the `path=` argument. | ||
* **Case 4**: File names using both the `path=` and `file_pattern=` arguments. |
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 summary is really helpful up top! It says the "what" of each case (e.g. case 3 is using the path argument). The piece that took me a bit of scanning to find is the "why" of each case. I wonder if we could give a very short statement either in the bullets or below to help readers understand why they might choose a given case.
It seems like case 1 and 2 are fairly obvious. But afaik for others it looks like the whys might be...
- case 3: specify image names relative to a base directory or url (e.g. "/path/to/images")
- case 4: specify image names inside a path (e.g. "metro_{}.svg")
I think you could get away with adding a sentence or two beneath the list explaining the whys of case 3 and 4 (or there might be some other nice way of including?)
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.
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 believe we still need to add a few sentences to explain the "why," with the flowchart serving as an additional aid. I'll continue working on this tomorrow, as it's quite late here.
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.
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 little call out box seems great!
docs/blog/rendering-images/index.qmd
Outdated
Our goal is to construct a Python dictionary to serve as input for creating a `Pandas` DataFrame. | ||
This dictionary will include the previously defined `data` and a new key, `"case1"`. The value for | ||
`"case1"` is generated by processing the `data["lines"]` column, which contains a list of strings. | ||
|
||
For each string in `data["lines"]`, we split it by commas to extract individual numbers. These | ||
numbers are then processed by adding the necessary prefix and suffix to generate valid file paths. | ||
Once processed, the paths are joined back into a single string using `", ".join()`. The resulting | ||
strings from all iterations are collected into a list, which becomes the value for `"case1"`: |
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.
These paragraphs seem to explain the code below them. Since that code is folded, I think you could get away with removing these, and instead provide a sentence preparing people for the table output.
(If it seems useful, you could put some of this explanation in the folded code block)
An example for explaining the table might be
Below is a DataFrame, whose
case1
column has full file paths that we want to turn into images.
docs/blog/rendering-images/index.qmd
Outdated
|
||
|
||
### Case 3: File Names with the `path=` Argument | ||
**Case 3** demonstrates how to simulate a column containing file names. The preparation for creating |
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.
A sentence on why might help prime readers for what they'll get out of this case. Maybe something like...
Case 3 demonstrates using the
path=
argument to specify images relative to a base directory. This lets you cut out a lot of repetition in file names. <remove DataFrame code prep explanation text>
docs/blog/rendering-images/index.qmd
Outdated
this works in **Case 4** below. | ||
|
||
### Case 4: File Names Using Both the `path=` and `file_pattern=` Arguments | ||
**Case 4** demonstrates how to simulate a column containing only the variable part of a string pattern. |
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.
Similar to case 3, the why of case 4 up top might help people see what they'll get out of this section. something like..?:
Case 4 demonstrates how to use
path=
andfile_pattern=
to specify images whose names are inside a common name. For example, you might usefile_pattern="metro_{}.svg"
, to specify"metro_1.svg"
,"metro_2.svg"
, etc..
@machow , thank you so much for the excellent suggestions! I’ll work on incorporating them into the post. @machow and @rich-iannone , I’d appreciate your thoughts on the following questions:
|
It seems okay to use, especially if it lets people see the file paths. It's tough the file names are so long, but also--since that's addressed by case study 3--maybe it's a chance to lean into the problem of the long file names. You could flag in case study 1, "these are filepaths to images inside the |
I think we ought to for these examples. |
@machow , thank you again for your excellent suggestions! I decided to place the "why" section in a callout to provide a quick tip for readers. I also refined the wording for each case, thanks to the helpful "something like" examples you provided. Additionally, I made the following updates:
@machow and @rich-iannone , feel free to make any direct updates to this PR if needed. |
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 blog post is incredible, @rich-iannone do you want to review? (or feel free to merge!)
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!!
Hello team,
I’d like to propose a blog post to help users better understand how to render images using the recent improvements to
GT.fmt_image()
andvals.fmt_image()
. Feedback is always welcome!In addition, I’ve noticed that the table alignment behaves differently depending on whether the
file_pattern=
argument is used. Below is a minimal example to demonstrate this behavior:I’ve attempted to investigate the code but couldn’t pinpoint the root cause. Could the team clarify if this is an intended feature or a potential bug?