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

Enhance GT.fmt_image() to support http/https schema in the columns parameter #520

Merged
merged 5 commits into from
Dec 10, 2024

Conversation

jrycw
Copy link
Collaborator

@jrycw jrycw commented Nov 23, 2024

Related PRs: #444, #451.

Currently, GT.fmt_image() supports formatting images from URLs by combining the columns and path parameters, as shown below:

import polars as pl
from great_tables import GT, vals, html

img_path = "https://raw.githubusercontent.com/posit-dev/great-tables/refs/heads/main/great_tables/data/metro_images/"

df = pl.DataFrame({"lines": ["tram_T6.svg", "tram_T7.svg", "tram_T8.svg"]})
GT(df).fmt_image("lines", path=img_path)

image


This PR proposes an enhancement to allow image formatting using only the columns parameter, enabling the following code to execute without errors:

df2 = df.with_columns(pl.lit(img_path).add(pl.col("lines")).alias("lines"))

image

GT(df2).fmt_image("lines")  # It produces the same table as the original approach.

This modification should benefit users who retrieve a list of URLs through a database query and store them as a column in the DataFrame. A possible scenario can be illustrated with the following pseudo-code:

...
urls: list[str] = db.execute("SELECT url FROM Foo")
df = pl.DataFrame({"urls": urls})
GT(df).fmt_image("urls")

Additionally, this change would make it more convenient for users who want to place images in locations other than the body using vals.fmt_image(). For example:

header_imgs = vals.fmt_image(f"{img_path}tram_T5.svg")
GT(df2).fmt_image("lines").tab_header(title=html(header_imgs[0]))

image


I believe this improvement will enhance the user experience and better align with the documentation, which states that complete http/https URLs should be accepted. That said, I'm open to any suggestions or feedback!

Copy link

codecov bot commented Nov 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.72%. Comparing base (4771178) to head (f76e2cb).
Report is 49 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #520   +/-   ##
=======================================
  Coverage   89.71%   89.72%           
=======================================
  Files          45       45           
  Lines        5202     5206    +4     
=======================================
+ Hits         4667     4671    +4     
  Misses        535      535           

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

@jrycw
Copy link
Collaborator Author

jrycw commented Nov 23, 2024

By the way, the new ruff settings appear to be working smoothly, and it seems our project runs on Python 3.13 without any issues.

Copy link
Collaborator

@machow machow left a comment

Choose a reason for hiding this comment

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

This LGTM, @rich-iannone do you want to merge?

@rich-iannone rich-iannone merged commit 03e38ab into posit-dev:main Dec 10, 2024
14 checks passed
@jrycw jrycw deleted the url-fmt_image branch December 11, 2024 01:04
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