docs/pyumya guides and deploy workflow - #19
Conversation
Summary of ChangesHello @wolfiesch, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request substantially enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
df1326b to
4c486e8
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly expands the documentation by adding several guides for pyumya features and improving the mkdocs configuration. The new guides are well-written and provide useful examples. My review focuses on improving the clarity and usability of the code examples in the new guides and refining the site navigation structure for a better user experience.
| ## Writing Conditional Formats | ||
|
|
||
| ### Cell Value Rules | ||
|
|
||
| ```python | ||
| book = UmyaBook() | ||
| book.add_sheet("Sales") | ||
|
|
||
| # Highlight cells greater than 1000 | ||
| book.add_conditional_format("Sales", { | ||
| "ranges": ["B2:B50"], | ||
| "type": "cellIs", | ||
| "operator": "greaterThan", | ||
| "formula": "1000", | ||
| "format": {"bg_color": "#C6EFCE", "font_color": "#006100"}, # green | ||
| }) | ||
|
|
||
| # Highlight cells below target | ||
| book.add_conditional_format("Sales", { | ||
| "ranges": ["B2:B50"], | ||
| "type": "cellIs", | ||
| "operator": "lessThan", | ||
| "formula": "500", | ||
| "format": {"bg_color": "#FFC7CE", "font_color": "#9C0006"}, # red | ||
| }) | ||
|
|
||
| book.save("output.xlsx") | ||
| ``` | ||
|
|
||
| ### Color Scales | ||
|
|
||
| ```python | ||
| # 2-color scale (red to green) | ||
| book.add_conditional_format("Sales", { | ||
| "ranges": ["C2:C50"], | ||
| "type": "colorScale", | ||
| "color_scale": { | ||
| "min_color": "#FF0000", | ||
| "max_color": "#00FF00", | ||
| }, | ||
| }) | ||
|
|
||
| # 3-color scale (red / yellow / green) | ||
| book.add_conditional_format("Sales", { | ||
| "ranges": ["D2:D50"], | ||
| "type": "colorScale", | ||
| "color_scale": { | ||
| "min_color": "#FF0000", | ||
| "mid_color": "#FFFF00", | ||
| "max_color": "#00FF00", | ||
| }, | ||
| }) | ||
| ``` | ||
|
|
||
| ### Data Bars | ||
|
|
||
| ```python | ||
| book.add_conditional_format("Sales", { | ||
| "ranges": ["E2:E50"], | ||
| "type": "dataBar", | ||
| "data_bar": {"color": "#638EC6"}, | ||
| }) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
The code examples under "Writing Conditional Formats" are structured in a way that could be confusing. The first example is a complete, runnable script that ends with book.save(), but the following examples for "Color Scales" and "Data Bars" are snippets that would fail if run independently. Given that UmyaBook.save() can only be called once per instance, this structure is misleading.
To improve clarity and user experience, I suggest combining all writing examples into a single, complete code block. This would demonstrate how to add multiple types of formatting in one go and ensure the code is copy-paste friendly. The subheadings for different rule types can be converted into comments within the code block for clarity.
## Writing Conditional Formats
The following is a complete example showing how to add various conditional formatting rules to a worksheet.
```python
from excelbench_rust import UmyaBook
book = UmyaBook()
book.add_sheet("Sales")
# --- Cell Value Rules ---
# Highlight cells greater than 1000
book.add_conditional_format("Sales", {
"ranges": ["B2:B50"],
"type": "cellIs",
"operator": "greaterThan",
"formula": "1000",
"format": {"bg_color": "#C6EFCE", "font_color": "#006100"}, # green
})
# Highlight cells below target
book.add_conditional_format("Sales", {
"ranges": ["B2:B50"],
"type": "cellIs",
"operator": "lessThan",
"formula": "500",
"format": {"bg_color": "#FFC7CE", "font_color": "#9C0006"}, # red
})
# --- Color Scales ---
# 2-color scale (red to green)
book.add_conditional_format("Sales", {
"ranges": ["C2:C50"],
"type": "colorScale",
"color_scale": {
"min_color": "#FF0000",
"max_color": "#00FF00",
},
})
# 3-color scale (red / yellow / green)
book.add_conditional_format("Sales", {
"ranges": ["D2:D50"],
"type": "colorScale",
"color_scale": {
"min_color": "#FF0000",
"mid_color": "#FFFF00",
"max_color": "#00FF00",
},
})
# --- Data Bars ---
book.add_conditional_format("Sales", {
"ranges": ["E2:E50"],
"type": "dataBar",
"data_bar": {"color": "#638EC6"},
})
book.save("output.xlsx")There was a problem hiding this comment.
Good call. Updated the guide to use a single runnable write example (single save) and aligned the payload/keys to the actual UmyaBook API. Fixed in 4c54a1d.
| ## Writing Validations | ||
|
|
||
| ### Dropdown List | ||
|
|
||
| ```python | ||
| book = UmyaBook() | ||
| book.add_sheet("Form") | ||
|
|
||
| # Create a dropdown list | ||
| book.add_data_validation("Form", { | ||
| "ranges": ["B2:B100"], | ||
| "type": "list", | ||
| "formula1": "Option A,Option B,Option C", | ||
| "show_dropdown": True, | ||
| }) | ||
|
|
||
| book.save("output.xlsx") | ||
| ``` | ||
|
|
||
| ### Numeric Constraints | ||
|
|
||
| ```python | ||
| # Whole number between 1 and 100 | ||
| book.add_data_validation("Form", { | ||
| "ranges": ["C2:C50"], | ||
| "type": "whole", | ||
| "operator": "between", | ||
| "formula1": "1", | ||
| "formula2": "100", | ||
| "error_title": "Invalid Input", | ||
| "error_message": "Enter a number between 1 and 100", | ||
| }) | ||
| ``` | ||
|
|
||
| ### Date Range | ||
|
|
||
| ```python | ||
| # Dates in 2026 only | ||
| book.add_data_validation("Form", { | ||
| "ranges": ["D2:D50"], | ||
| "type": "date", | ||
| "operator": "between", | ||
| "formula1": "2026-01-01", | ||
| "formula2": "2026-12-31", | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
The code examples under "Writing Validations" could be more user-friendly. The first example for "Dropdown List" is a complete script with book.save(), but the subsequent "Numeric Constraints" and "Date Range" examples are incomplete snippets. This is confusing because UmyaBook.save() can only be called once.
I recommend combining these into a single, runnable code block to show how multiple validations can be added to a sheet before saving. This makes the documentation clearer and easier for users to follow.
## Writing Validations
The following is a complete example showing how to add various data validation rules to a worksheet.
```python
from excelbench_rust import UmyaBook
book = UmyaBook()
book.add_sheet("Form")
# --- Dropdown List ---
book.add_data_validation("Form", {
"ranges": ["B2:B100"],
"type": "list",
"formula1": "Option A,Option B,Option C",
"show_dropdown": True,
})
# --- Numeric Constraints ---
# Whole number between 1 and 100
book.add_data_validation("Form", {
"ranges": ["C2:C50"],
"type": "whole",
"operator": "between",
"formula1": "1",
"formula2": "100",
"error_title": "Invalid Input",
"error_message": "Enter a number between 1 and 100",
})
# --- Date Range ---
# Dates in 2026 only
book.add_data_validation("Form", {
"ranges": ["D2:D50"],
"type": "date",
"operator": "between",
"formula1": "2026-01-01",
"formula2": "2026-12-31",
})
book.save("output.xlsx")There was a problem hiding this comment.
Agreed. Combined the write examples into one runnable block (single save) and updated to the binding's dict keys. Fixed in 4c54a1d.
| # Freeze top row (headers stay visible) | ||
| book.set_freeze_panes("Data", {"row": 1, "column": 0}) | ||
|
|
||
| # Freeze first column | ||
| book.set_freeze_panes("Data", {"row": 0, "column": 1}) | ||
|
|
||
| # Freeze both (top-left corner stays fixed) | ||
| book.set_freeze_panes("Data", {"row": 1, "column": 1}) |
There was a problem hiding this comment.
The code example for writing freeze panes is a bit misleading. It shows three consecutive calls to set_freeze_panes for the same sheet. Since each call will overwrite the previous one, only the last setting ({"row": 1, "column": 1}) will be applied when the workbook is saved.
To make it clear that these are alternative options, I suggest commenting out the first two examples and leaving the last one active. This shows the user different possibilities while providing a runnable example.
| # Freeze top row (headers stay visible) | |
| book.set_freeze_panes("Data", {"row": 1, "column": 0}) | |
| # Freeze first column | |
| book.set_freeze_panes("Data", {"row": 0, "column": 1}) | |
| # Freeze both (top-left corner stays fixed) | |
| book.set_freeze_panes("Data", {"row": 1, "column": 1}) | |
| # To freeze the top row (headers stay visible): | |
| # book.set_freeze_panes("Data", {"row": 1, "column": 0}) | |
| # To freeze the first column: | |
| # book.set_freeze_panes("Data", {"row": 0, "column": 1}) | |
| # To freeze both (top-left corner stays fixed): | |
| book.set_freeze_panes("Data", {"row": 1, "column": 1}) |
There was a problem hiding this comment.
Good catch. Updated the guide to treat the examples as alternatives (only one active call) and aligned the settings dict to mode/top_left_cell. Fixed in 4c54a1d.
| - Getting Started: | ||
| - Installation: index.md |
There was a problem hiding this comment.
The navigation structure has a redundant entry. Both "Home" and the nested "Installation" under "Getting Started" point to index.md. This will create two top-level navigation items pointing to the same page, which can be confusing for users.
Since index.md serves as the main landing page with installation instructions, the "Getting Started" section is redundant. I recommend removing it to simplify the navigation.
There was a problem hiding this comment.
Yep. Removed the duplicate nav entry so index.md only appears once in the nav. Fixed in 4c54a1d.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - Home: index.md | ||
| - API Reference: api-reference.md | ||
| - Limitations: limitations.md | ||
| # Guides added as Phase 1 features land: | ||
| # - Merged Cells: guides/merged-cells.md | ||
| # - Comments: guides/comments.md | ||
| # - Hyperlinks: guides/hyperlinks.md | ||
| # etc. | ||
| - Getting Started: | ||
| - Installation: index.md |
There was a problem hiding this comment.
Duplicate index.md in nav
Both "Home" (line 67) and "Getting Started > Installation" (line 69) point to index.md. This creates a duplicate entry in the navigation — clicking either link renders the same page. With navigation.indexes enabled, mkdocs-material will also emit a build warning about the duplicate.
Consider either giving "Getting Started > Installation" its own dedicated page (e.g., getting-started.md) or removing the "Getting Started" section entirely since the installation instructions already live on the Home page.
| - Home: index.md | |
| - API Reference: api-reference.md | |
| - Limitations: limitations.md | |
| # Guides added as Phase 1 features land: | |
| # - Merged Cells: guides/merged-cells.md | |
| # - Comments: guides/comments.md | |
| # - Hyperlinks: guides/hyperlinks.md | |
| # etc. | |
| - Getting Started: | |
| - Installation: index.md | |
| - Home: index.md | |
| - Guides: |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/pyumya/mkdocs.yml
Line: 67:69
Comment:
**Duplicate `index.md` in nav**
Both "Home" (line 67) and "Getting Started > Installation" (line 69) point to `index.md`. This creates a duplicate entry in the navigation — clicking either link renders the same page. With `navigation.indexes` enabled, mkdocs-material will also emit a build warning about the duplicate.
Consider either giving "Getting Started > Installation" its own dedicated page (e.g., `getting-started.md`) or removing the "Getting Started" section entirely since the installation instructions already live on the Home page.
```suggestion
- Home: index.md
- Guides:
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Agreed. Dropped the redundant Getting Started > Installation entry (kept Home as index.md). Fixed in 4c54a1d.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df1326bd7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from excelbench_rust import UmyaBook | ||
|
|
||
| book = UmyaBook.open("report.xlsx") | ||
| merged = book.read_merged_cells("Sheet1") |
There was a problem hiding this comment.
The guide calls book.read_merged_cells("Sheet1"), but UmyaBook exposes read_merged_ranges (see rust/excelbench_rust/src/umya/merged_cells.rs, pub fn read_merged_ranges). Users following this snippet will hit AttributeError immediately, so the first read example in this page is not runnable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated the guide to call UmyaBook.read_merged_ranges (exported API name) and clarified unmerge is not exposed. Fixed in 4c54a1d.
| book.add_sheet("Data") | ||
|
|
||
| book.write_cell_value("Data", "A1", {"type": "number", "value": 42.0}) | ||
| book.add_comment("Data", "A1", { |
There was a problem hiding this comment.
This example passes three positional arguments to add_comment, but the binding takes only (sheet, comment_dict) and requires cell inside the dict (rust/excelbench_rust/src/umya/comments.rs, pub fn add_comment). As written, the snippet raises a Python argument error before writing anything.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated add_comment example to match the binding signature (sheet, dict with required cell). Fixed in 4c54a1d.
| "ranges": ["B2:B100"], | ||
| "type": "list", |
There was a problem hiding this comment.
The payload uses ranges and type, but the Rust binding reads range and validation_type (rust/excelbench_rust/src/umya/data_validation.rs, get_item("range") / get_item("validation_type")). Copying this example leaves the validation target/type unset, so the generated rule will not match what the guide describes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated the examples to use range + validation_type (matching add_data_validation). Fixed in 4c54a1d.
| book.add_sheet("Data") | ||
|
|
||
| # Freeze top row (headers stay visible) | ||
| book.set_freeze_panes("Data", {"row": 1, "column": 0}) |
There was a problem hiding this comment.
The write example uses {"row": ..., "column": ...}, but set_freeze_panes parses mode plus top_left_cell/split fields (rust/excelbench_rust/src/umya/freeze_panes.rs), not row/column. With this payload, users won't actually configure the freeze state shown in the guide.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated freeze pane docs to match the supported settings dict (mode + top_left_cell / split fields). Fixed in 4c54a1d.
| book = UmyaBook.open("dashboard.xlsx") | ||
| rules = book.read_conditional_formats("Sheet1") | ||
| for r in rules: | ||
| print(f"{r['ranges']}: {r['type']}") |
There was a problem hiding this comment.
The read loop indexes r['ranges'] and r['type'], but read_conditional_formats returns range and rule_type (rust/excelbench_rust/src/umya/conditional_fmt.rs). Running this snippet raises KeyError, so the documented read path for this feature is broken.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated read example to use the API-returned keys (range, rule_type). Fixed in 4c54a1d.
There was a problem hiding this comment.
Pull request overview
This PR updates the pyumya documentation site configuration and adds a set of new “Guides” pages, while also adjusting the GitHub Actions deploy workflow to trigger on changes to deploy/index.html and to change the Vercel deploy invocation.
Changes:
- Expanded MkDocs Material configuration (navigation features, plugins, social/repo icons) and reorganized navigation to include a Guides section.
- Added multiple new pyumya guide pages (merged cells, comments, hyperlinks, freeze panes, images, data validation, conditional formatting).
- Updated the dashboard deploy workflow trigger paths and the Vercel deploy command.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/pyumya/mkdocs.yml | Enables additional MkDocs Material features/plugins and restructures nav to surface new Guides/Reference sections. |
| docs/pyumya/docs/guides/merged-cells.md | Adds merged-cells guide content (but examples currently don’t match the binding API). |
| docs/pyumya/docs/guides/comments.md | Adds comments guide content (but write example signature doesn’t match the binding API). |
| docs/pyumya/docs/guides/hyperlinks.md | Adds hyperlinks guide content (but write example signature doesn’t match the binding API). |
| docs/pyumya/docs/guides/freeze-panes.md | Adds freeze panes guide content (but read/write shapes don’t match the binding API). |
| docs/pyumya/docs/guides/images.md | Adds images guide content (but read/write shapes don’t match the binding API). |
| docs/pyumya/docs/guides/data-validation.md | Adds data validation guide content (but field names don’t match the binding API). |
| docs/pyumya/docs/guides/conditional-formatting.md | Adds conditional formatting guide content (but return/payload keys and supported fields don’t match the binding API). |
| .github/workflows/deploy-dashboard.yml | Triggers on deploy/index.html changes and adjusts how Vercel deploy is executed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Merge and unmerge cell ranges in Excel workbooks. | ||
|
|
||
| ## Reading Merged Ranges | ||
|
|
||
| ```python | ||
| from excelbench_rust import UmyaBook | ||
|
|
||
| book = UmyaBook.open("report.xlsx") | ||
| merged = book.read_merged_cells("Sheet1") | ||
| print(merged) # ["A1:D1", "B3:B5"] |
There was a problem hiding this comment.
The guide uses a non-existent API and claims unmerge support. The Rust binding exposes UmyaBook.read_merged_ranges(sheet) (not read_merged_cells), and there is no unmerge method in the binding—please update the text and example so it matches the actual public API.
There was a problem hiding this comment.
Updated merged-cells guide to the actual API (read_merged_ranges) and removed the unmerge claim (added a note instead). Fixed in 4c54a1d.
|
|
||
| book = UmyaBook.open("dashboard.xlsx") | ||
| panes = book.read_freeze_panes("Sheet1") | ||
| print(panes) # {"row": 1, "column": 0} (top row frozen) |
There was a problem hiding this comment.
read_freeze_panes returns a dict describing the pane state (e.g., mode, top_left_cell, and for split panes x_split/y_split), not row/column. The example output shape here doesn’t match the binding and will confuse readers—please update it to reflect the actual return value.
| print(panes) # {"row": 1, "column": 0} (top row frozen) | |
| print(panes) # {"mode": "frozen", "top_left_cell": "A2", "x_split": 0, "y_split": 1} (top row frozen) |
There was a problem hiding this comment.
Updated read_freeze_panes example to reflect the actual return shape (mode/top_left_cell/etc.). Fixed in 4c54a1d.
| # Freeze top row (headers stay visible) | ||
| book.set_freeze_panes("Data", {"row": 1, "column": 0}) | ||
|
|
||
| # Freeze first column | ||
| book.set_freeze_panes("Data", {"row": 0, "column": 1}) | ||
|
|
||
| # Freeze both (top-left corner stays fixed) | ||
| book.set_freeze_panes("Data", {"row": 1, "column": 1}) | ||
|
|
There was a problem hiding this comment.
set_freeze_panes expects a settings dict with at least mode (e.g., freeze/split) and typically top_left_cell (e.g., "B2"). The examples use {row, column} which the binding does not read, so these calls will not produce the intended result—please adjust to the supported settings keys.
There was a problem hiding this comment.
Updated set_freeze_panes examples to use mode/top_left_cell (and clarified overwrite semantics). Fixed in 4c54a1d.
| rules = book.read_conditional_formats("Sheet1") | ||
| for r in rules: | ||
| print(f"{r['ranges']}: {r['type']}") | ||
| # ['A1:A100']: cellIs | ||
| # ['B1:B100']: colorScale |
There was a problem hiding this comment.
read_conditional_formats returns items with keys like range and rule_type (plus optional operator, formula, etc.). The example uses r['ranges']/r['type'], which will raise KeyError—please align the guide with the binding’s actual return shape.
There was a problem hiding this comment.
Aligned conditional format read payload keys to range/rule_type (+ operator/formula/format). Fixed in 4c54a1d.
| book.add_conditional_format("Sales", { | ||
| "ranges": ["B2:B50"], | ||
| "type": "cellIs", | ||
| "operator": "greaterThan", | ||
| "formula": "1000", | ||
| "format": {"bg_color": "#C6EFCE", "font_color": "#006100"}, # green | ||
| }) |
There was a problem hiding this comment.
add_conditional_format expects a single dict with keys like range and rule_type (not ranges/type). As written, this example won’t match the binding’s expected input payload, so it will either error or silently omit settings—please update the payload keys to the supported names.
There was a problem hiding this comment.
Aligned add_conditional_format payload to range/rule_type (not ranges/type). Fixed in 4c54a1d.
| book.add_sheet("Data") | ||
|
|
||
| book.write_cell_value("Data", "A1", {"type": "number", "value": 42.0}) | ||
| book.add_comment("Data", "A1", { |
There was a problem hiding this comment.
UmyaBook.add_comment in the Rust binding takes (sheet, comment_dict) where comment_dict includes a required cell field. The example currently passes the cell as a separate positional argument, which will raise a Python argument error—please adjust the example payload shape to match the actual signature.
| book.add_comment("Data", "A1", { | |
| book.add_comment("Data", { | |
| "cell": "A1", |
There was a problem hiding this comment.
Updated add_comment usage to pass a dict including cell/text/author (no extra positional arg). Fixed in 4c54a1d.
| book.add_hyperlink("Links", "A1", { | ||
| "target": "https://example.com", | ||
| "display": "Visit Example", | ||
| }) | ||
|
|
||
| # Email link | ||
| book.add_hyperlink("Links", "A2", { | ||
| "target": "mailto:support@example.com", | ||
| "display": "Email Support", | ||
| }) | ||
|
|
||
| # Internal reference (another sheet) | ||
| book.add_hyperlink("Links", "A3", { |
There was a problem hiding this comment.
UmyaBook.add_hyperlink is defined as (sheet, link_dict) with required cell and target keys (plus optional display, tooltip, internal). The example currently passes the cell as a separate positional argument, which doesn't match the binding’s signature and will fail at runtime.
| book.add_hyperlink("Links", "A1", { | |
| "target": "https://example.com", | |
| "display": "Visit Example", | |
| }) | |
| # Email link | |
| book.add_hyperlink("Links", "A2", { | |
| "target": "mailto:support@example.com", | |
| "display": "Email Support", | |
| }) | |
| # Internal reference (another sheet) | |
| book.add_hyperlink("Links", "A3", { | |
| book.add_hyperlink("Links", { | |
| "cell": "A1", | |
| "target": "https://example.com", | |
| "display": "Visit Example", | |
| }) | |
| # Email link | |
| book.add_hyperlink("Links", { | |
| "cell": "A2", | |
| "target": "mailto:support@example.com", | |
| "display": "Email Support", | |
| }) | |
| # Internal reference (another sheet) | |
| book.add_hyperlink("Links", { | |
| "cell": "A3", |
There was a problem hiding this comment.
Updated add_hyperlink examples to match the binding signature (sheet, dict with required cell/target). Fixed in 4c54a1d.
| print(f"Cell {img['cell']}: {img['format']} ({len(img['data'])} bytes)") | ||
| # Cell A1: png (24576 bytes) |
There was a problem hiding this comment.
UmyaBook.read_images currently returns anchor metadata (e.g., cell, anchor, offset, etc.) and does not include image bytes or a format field. The example iterates img['format'] and img['data'], which will raise KeyError—please update the example to match the actual return shape.
| print(f"Cell {img['cell']}: {img['format']} ({len(img['data'])} bytes)") | |
| # Cell A1: png (24576 bytes) | |
| print(f"Image at {img['cell']}: anchor={img['anchor']}, offset={img['offset']}") | |
| # Image at A1: anchor=oneCell, offset=(0, 0) |
There was a problem hiding this comment.
Updated read_images example to match the current return shape (anchor metadata; no bytes/format). Fixed in 4c54a1d.
| # Embed an image from file | ||
| book.add_image("Report", "B2", { | ||
| "data": Path("logo.png").read_bytes(), | ||
| "format": "png", |
There was a problem hiding this comment.
UmyaBook.add_image expects an image dict containing a file path and cell (it does not accept raw data bytes or a format field). As written, this example will fail input validation in the binding—please update the example to pass path/cell (and adjust surrounding text accordingly).
| # Embed an image from file | |
| book.add_image("Report", "B2", { | |
| "data": Path("logo.png").read_bytes(), | |
| "format": "png", | |
| # Embed an image by file path | |
| book.add_image("Report", { | |
| "path": "logo.png", | |
| "cell": "B2", |
There was a problem hiding this comment.
Updated add_image example to match the binding signature (sheet, dict with path + cell). Fixed in 4c54a1d.
| validations = book.read_data_validations("Sheet1") | ||
| for v in validations: | ||
| print(f"{v['ranges']}: {v['type']} — {v.get('formula1', '')}") | ||
| # ['B2:B100']: list — "Option A,Option B,Option C" | ||
| # ['C2:C100']: whole — 1 | ||
| ``` |
There was a problem hiding this comment.
The data validation examples use field names that don’t match the Rust binding. read_data_validations returns keys like range and validation_type (and error, not error_message), and add_data_validation expects the same (range, validation_type, etc.), whereas the guide uses ranges/type and other non-existent keys.
There was a problem hiding this comment.
Updated both read and write examples to match the binding keys (range, validation_type, error, etc.). Fixed in 4c54a1d.
Greptile Overview
Greptile Summary
This PR makes two changes: (1) fixes the Vercel deploy workflow by switching from
vercel deploy deploy/tovercel deploy --cwd deployand addingdeploy/index.htmlas a trigger path, and (2) adds seven new pyumya documentation guides covering Phase 1 features (merged cells, comments, hyperlinks, freeze panes, images, data validation, conditional formatting) along with mkdocs navigation improvements (minify plugin, emoji support, social links, restructured nav).--cwdflag is the correct way to specify the deploy directory for newer Vercel CLI versions. Addingdeploy/index.htmlto trigger paths ensures manual dashboard edits also trigger deployment.index.mdandlimitations.md.minify,emoji,attr_list,md_in_html). Themkdocs-minify-plugindependency is already declared inpyproject.toml.index.mdis referenced twice in the nav (as "Home" and as "Getting Started > Installation"), creating a redundant navigation entry.Confidence Score: 4/5
docs/pyumya/mkdocs.ymlhas a duplicateindex.mdreference in the nav section that should be addressed.Important Files Changed
deploy/index.htmlto trigger paths and fixes Vercel CLI invocation fromvercel deploy deploy/tovercel deploy --cwd deploy. Both changes are correct.index.mdreference in nav (Home and Getting Started > Installation point to same page).Flowchart
flowchart TD A["mkdocs.yml nav"] --> B["Home\n(index.md)"] A --> C["Getting Started"] C --> D["Installation\n(index.md) ⚠️ duplicate"] A --> E["Guides"] E --> F["Merged Cells"] E --> G["Comments"] E --> H["Hyperlinks"] E --> I["Freeze Panes"] E --> J["Images"] E --> K["Data Validation"] E --> L["Conditional Formatting"] A --> M["Reference"] M --> N["API Reference"] M --> O["Limitations"] style D fill:#fff3cd,stroke:#ffc107Last reviewed commit: 4c486e8