Add table() method for formatted HTML table visualization#1083
Add table() method for formatted HTML table visualization#1083nitininhouse wants to merge 1 commit intofossasia:masterfrom
Conversation
There was a problem hiding this comment.
Sorry @nitininhouse, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
There was a problem hiding this comment.
Pull request overview
Adds a new table() helper to the Visdom Python client to render structured data as a styled HTML table via the existing text() endpoint.
Changes:
- Introduces
Visdom.table(headers, data, ...)to generate HTML<table>markup from headers + rows - Adds basic table styling (header background, alternating row colors, monospace font)
- Performs basic input validation and sets a default window title
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| header_html = ''.join( | ||
| '<th style="background:#4a90d9; color:white; ' | ||
| 'padding:8px 12px; text-align:left;">%s</th>' % str(h) | ||
| for h in headers | ||
| ) |
There was a problem hiding this comment.
Header and cell values are interpolated into HTML without escaping. If any values contain HTML (e.g., <script>), this will be rendered/executed in the Visdom UI (XSS risk). Escape h and cell with html.escape(str(...), quote=True) (or an equivalent escaping utility already used in the project) before inserting into the <th>/<td> markup.
| cells = ''.join( | ||
| '<td style="padding:6px 12px; ' | ||
| 'border-bottom:1px solid #ddd;">%s</td>' % str(cell) | ||
| for cell in row | ||
| ) |
There was a problem hiding this comment.
Header and cell values are interpolated into HTML without escaping. If any values contain HTML (e.g., <script>), this will be rendered/executed in the Visdom UI (XSS risk). Escape h and cell with html.escape(str(...), quote=True) (or an equivalent escaping utility already used in the project) before inserting into the <th>/<td> markup.
| assert isinstance(headers, list), "headers should be a list" | ||
| assert isinstance(data, list), "data should be a list of lists" | ||
| for row in data: | ||
| assert isinstance(row, list), "each row in data should be a list" | ||
| assert len(row) == len(headers), \ | ||
| "each row length must match number of headers" |
There was a problem hiding this comment.
Using assert for runtime input validation is brittle because asserts can be disabled with Python optimizations (-O), which would remove these checks in production. Prefer raising TypeError/ValueError with the same messages (or more specific ones) so validation is enforced consistently.
| assert isinstance(headers, list), "headers should be a list" | |
| assert isinstance(data, list), "data should be a list of lists" | |
| for row in data: | |
| assert isinstance(row, list), "each row in data should be a list" | |
| assert len(row) == len(headers), \ | |
| "each row length must match number of headers" | |
| if not isinstance(headers, list): | |
| raise TypeError("headers should be a list") | |
| if not isinstance(data, list): | |
| raise TypeError("data should be a list of lists") | |
| for row in data: | |
| if not isinstance(row, list): | |
| raise TypeError("each row in data should be a list") | |
| if len(row) != len(headers): | |
| raise ValueError("each row length must match number of headers") |
| # Build rows HTML | ||
| rows_html = '' | ||
| for i, row in enumerate(data): | ||
| bg = '#fafafa' if i % 2 == 0 else '#ffffff' | ||
| cells = ''.join( | ||
| '<td style="padding:6px 12px; ' | ||
| 'border-bottom:1px solid #ddd;">%s</td>' % str(cell) | ||
| for cell in row | ||
| ) | ||
| rows_html += ( | ||
| '<tr style="background:%s;">%s</tr>' % (bg, cells) | ||
| ) |
There was a problem hiding this comment.
Building rows_html via repeated string concatenation in a loop is O(n²) in total copied characters for large tables. Use a list accumulator (append each <tr>...</tr>) and ''.join(rows) at the end to keep it O(n) and reduce allocations.
| table_html = ( | ||
| '<table style="border-collapse:collapse; ' | ||
| 'width:100%%; font-family:monospace;">' | ||
| '<thead><tr>%s</tr></thead>' | ||
| '<tbody>%s</tbody>' | ||
| '</table>' | ||
| ) % (header_html, rows_html) |
There was a problem hiding this comment.
The PR description mentions 'hover effects', but the generated markup only sets static inline styles (no :hover styling and no JS). Either update the description to match the current implementation or add a hover style (e.g., via embedded <style> and a class-based selector) so the feature matches the stated behavior.
|
|
||
| ) | ||
|
|
There was a problem hiding this comment.
There appears to be an extra blank line/trailing whitespace introduced around the end of the preceding call. Please remove trailing whitespace and keep formatting consistent to avoid lint/format churn in future diffs.
| ) | |
| ) |
|
Hello @mariobehling @marcoag @norbusan |
Description
Added
table()method to Visdom for displaying formatted HTML tables. Renders structured data with styled headers, alternating row colors, hover effects, and monospace font for clean presentation.Motivation and Context
Tables are essential for displaying structured training metrics, results summaries, and comparison data. This feature eliminates manual HTML formatting and provides a consistent, visually appealing way to visualize tabular data within Visdom.
closes #1081
How Has This Been Tested?
Tested with sample training data: