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

Add GT.write_html() as a helper function for easier HTML output #485

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

Conversation

jrycw
Copy link
Collaborator

@jrycw jrycw commented Oct 5, 2024

Hello team,

I've noticed that some users expect an easier way to interact with our tables. While GT.as_raw_html() is great, it doesn’t fully meet those expectations. To address this, I’d like to propose adding GT.write_html() as a helper function, simplifying the process of writing the table’s HTML directly to a file without needing to use open().

I believe this is a useful addition, and if necessary, we could even rename it to GT._write_html() for unofficial use.

Copy link

codecov bot commented Oct 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.89%. Comparing base (f1c8c94) to head (d32044b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #485      +/-   ##
==========================================
+ Coverage   88.88%   88.89%   +0.01%     
==========================================
  Files          44       44              
  Lines        5216     5225       +9     
==========================================
+ Hits         4636     4645       +9     
  Misses        580      580              

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

@jrycw
Copy link
Collaborator Author

jrycw commented Oct 15, 2024

I've found that utilizing the parameters of GT.as_raw_html() might be a better approach. I suggest we keep the make_page and all_important parameters undocumented, similar to how it's done in GT.as_raw_html().

@machow
Copy link
Collaborator

machow commented Nov 12, 2024

Thanks for this @jrycw -- @rich-iannone and I are pairing on this right now, and thinking about the use of both .as_*() and .write_*() in the Great Tables API.

We noticed that polars .write_*() methods will return a string, if not filename is specified. WDYT of that approach. So in that case, we would deprecate the as_*() methods in favor of write_*() methods.

Basically, it seems like there are 3 possible approaches:

  • polars: .write_*() methods return a string when no filename given
  • two kinds of methods: each render target has a .as_*() and .write_*() method.
  • polars approach, but with .as_*(): use .as_*() instead of .write_*()

Would love to get your input!

@jrycw
Copy link
Collaborator Author

jrycw commented Nov 13, 2024

Hello team,

I’m inclined to go with the first option as the higher-level abstraction while retaining .as_*() for internal use.

Here’s my draft implementation below.

@jrycw
Copy link
Collaborator Author

jrycw commented Nov 13, 2024

To issue a deprecation message to users, we could take a similar approach to what Polars uses. We might consider decorating as_raw_html() in the GT object, like this:

class GT(GTData):
    ...

    as_raw_html = deprecate_function("Use `GT.write_html()` instead.", version="0.15.0")(as_raw_html) 
    write_html = write_html

This method avoids directly decorating as_raw_html(), which could trigger the message unintentionally when using write_html(). Once we’re ready for full deprecation, we can simply remove the line: as_raw_html = ....

@machow machow self-requested a review November 13, 2024 21:31
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.

2 participants