Skip to content

makefile(streamlit the same, stlite added, bug for display fixed, out…#21

Open
EddW1219 wants to merge 2 commits intomainfrom
edward_branch
Open

makefile(streamlit the same, stlite added, bug for display fixed, out…#21
EddW1219 wants to merge 2 commits intomainfrom
edward_branch

Conversation

@EddW1219
Copy link
Copy Markdown
Collaborator

@EddW1219 EddW1219 commented Mar 25, 2026 •
edited by Copilot AI
Loading

  • Fix running_in_stlite(): remove duplicate, use Pyodide-recommended detection (sys.platform == "emscripten")
  • Escape HTML labels in utils/parameter_ui.py with html.escape() to prevent injection
  • Fix Makefile: UV ?= uv, fix smelected typo, restructure $(BUILD_DIR) mkdir into INDEX_HTML rule
  • Remove -> None from main() in scripts/build_index.py
  • Fix AGENTS.md function contract for render_parameters_with_indent
  • Fix Excel reset callback model_id mismatch in app.py

Copilot AI review requested due to automatic review settings March 25, 2026 07:27
Copilot AI reviewed Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds stlite build scaffolding and contributor guidance while adjusting parameter loading/rendering to better support stlite (including handling nested YAML parameters: blocks and attempting to prevent blank widget values from overwriting defaults).

Changes:

  • Add stlite build generation via scripts/build_index.py and a new Makefile workflow.
  • Normalize YAML parameter defaults and add helpers for leaf-parameter defaults/coercion (stlite-specific).
  • Add AGENTS.md contributor/agent conventions and refactor parameter UI rendering/reset behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
utils/parameter_ui.py Refactors parameter reset/render logic and changes widget keying + sidebar rendering.
utils/parameter_loader.py Adds typing, supports nested parameters: YAML layouts, and introduces get_leaf_defaults.
scripts/build_index.py New helper to generate build/index.html for stlite and mount project files.
app.py Adds stlite detection + parameter normalization/coercion; updates parameter source handling and UI flows.
Makefile New targets for dev/stlite build/serve and quality checks.
AGENTS.md New contributor/agent conventions and documented function contracts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gvegayon gvegayon requested a review from olivia-banks March 25, 2026 16:07
@gvegayon
Copy link
Copy Markdown
Member

gvegayon commented Mar 25, 2026

Thank you, @EddW1219! @olivia-banks, can you take a look at this PR?

olivia-banks
olivia-banks requested changes Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@olivia-banks olivia-banks left a comment

Choose a reason for hiding this comment

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

👍!

@olivia-banks olivia-banks mentioned this pull request Mar 25, 2026
Open
10 tasks
@olivia-banks
Copy link
Copy Markdown
Member

olivia-banks commented Mar 26, 2026

Linking to #13.

@olivia-banks
Copy link
Copy Markdown
Member

olivia-banks commented Mar 31, 2026

@EddW1219 Do you have an ETA on this?

@gvegayon
Copy link
Copy Markdown
Member

gvegayon commented Mar 31, 2026

@copilot, please address the comments in #21 (review) and #21 (review)

attn @olivia-banks @EddW1219

Copilot AI requested a review from gvegayon March 31, 2026 20:23
@olivia-banks
Copy link
Copy Markdown
Member

olivia-banks commented Apr 1, 2026 •
edited
Loading

@gvegayon I think this is okay to be merged as is. A fair number of these are rendered obsolete by #17. The failing action is already fixed in main as well.

@EddW1219
chore: add type stubs and dev dependencies
50b18e2 
- Add pandas-stubs, types-openpyxl, types-PyYAML to dev dependencies
- All checks now pass: ruff, mypy, pytest
- 91% coverage on utils/parameter_ui.py (changed code)
- Addresses olivia-banks' review comments
@EddW1219 EddW1219 force-pushed the edward_branch branch 2 times, most recently from 8b8e87f to 50b18e2 Compare April 1, 2026 02:38
@EddW1219
Copy link
Copy Markdown
Collaborator Author

EddW1219 commented Apr 1, 2026

@gvegayon @olivia-banks Hi George and Olivia, I just merged the changes to current PR.

@olivia-banks
Copy link
Copy Markdown
Member

olivia-banks commented Apr 1, 2026 •
edited
Loading

This looks great to me, and I think it's ready to merge after we get #17 in here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@olivia-banks olivia-banks olivia-banks requested changes

@gvegayon gvegayon Awaiting requested review from gvegayon

+1 more reviewer

Copilot code review Copilot Copilot left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

@EddW1219 EddW1219

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@EddW1219 @gvegayon @olivia-banks