8000 Partial fix for 1078: [refactor: Simplify HTML generation in PyDataFrame by extracting helper functions] by kosiew · Pull Request #1087 · apache/datafusion-python · GitHub
[go: up one dir, main page]

Skip to content

Partial fix for 1078: [refactor: Simplify HTML generation in PyDataFrame by extracting helper functions] #1087

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

Closed
wants to merge 1 commit into from

Conversation

kosiew
Copy link
Contributor
@kosiew kosiew commented Mar 28, 2025

Which issue does this PR close?

Partial fix for #1078

Rationale for this change

Split up some of the html generation into a set of helper functions.

The rendering logic for DataFrame HTML output was previously monolithic. By modularizing the generation of HTML structure, styles, and table rows into clearly named helper functions, we improve readability, testability, and long-term maintainability.

This refactor lays the foundation for easier customization.

What changes are included in this PR?

  • Extracted style definitions into get_html_style_definitions()
  • Moved the opening table tag into get_html_table_opening()
  • Separated table header generation into get_html_table_header()
  • Encapsulated array formatter creation logic in create_batch_formatters()
  • Delegated row and cell HTML generation to get_html_table_rows() and format_table_cell()
  • Moved JavaScript logic for expandable cells into get_html_js_functions()
  • Preserved original behavior with no changes to the rendered output

Are there any user-facing changes?

No functional changes for end users. The generated HTML and behavior remain the same, but the underlying implementation is now modular and easier to maintain.

…ions

- Replaced inline HTML string construction with dedicated functions for style definitions, table headers, rows, and cell formatting.
- Improved readability and maintainability of the code by modularizing HTML generation logic.
- Enhanced the handling of large cell content with expandable text features.
@timsaucer
Copy link
Contributor

Thinking about this and also #1086 does it make sense instead to have a global variable like in utils.rs has get_global_ctx()? It would simplify a lot of what's going on and I think we also want the option to disable the formatting we're applying in these functions.

I don't have a perfect use case in my mind, but I can imagine a user might want to be able to do either

  • keep the format options we have hard coded
  • assign a style that matches their css elsewhere
  • just disable all formatting

@kosiew
Copy link
Contributor Author
kosiew commented Apr 7, 2025

@timsaucer

want the option to disable the formatting we're applying in these functions....

Do you mean something like #1096?

@timsaucer
Copy link
Contributor

@timsaucer

want the option to disable the formatting we're applying in these functions....

Do you mean something like #1096?

Yes, I think that issue captures it really well.

@kosiew
Copy link
Contributor Author
kosiew commented Apr 8, 2025

Thanks for the confirmation, @timsaucer

Closing this to pivot to #1096

@kosiew kosiew closed this Apr 8, 2025
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
0