Skip to content

chore: improve Meshroom maintenance path#3125

Open
lphuc2250gma wants to merge 1 commit into
alicevision:developfrom
lphuc2250gma:maint/20260605150510
Open

chore: improve Meshroom maintenance path#3125
lphuc2250gma wants to merge 1 commit into
alicevision:developfrom
lphuc2250gma:maint/20260605150510

Conversation

@lphuc2250gma

Copy link
Copy Markdown

Summary:

  • Add or tighten focused edge-case tests or type assertions in tests/utils.py, tests/test_graph.py related to Python typing, tests, CLI ergonomics, observability; avoid docs-only changes and broad refactors.
  • Keep the change narrow so it is straightforward to review.

Notes:

  • I kept this scoped to the relevant implementation and tests.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds a new test case for empty graphs in tests/test_graph.py and updates type hints in tests/utils.py to use type[desc.Node] instead of desc.Node. A review comment points out that comparing graph.nodes (a DictModel instance) directly to an empty list [] in the new test will fail, and suggests asserting its length instead.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tests/test_graph.py
def test_empty_graph():
"""Test edge-case behavior on a graph with no nodes."""
graph = Graph("")
assert graph.nodes == []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Comparing graph.nodes (which is a DictModel instance) directly to a list [] will evaluate to False because DictModel is a custom container class and not a standard Python list. To check if the graph has no nodes, you should check its length or use its truthiness.

Suggested change
assert graph.nodes == []
assert len(graph.nodes) == 0

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.

1 participant