Skip to content

Add regression test for numpy_num_face_vertices() fix (issue #400)#418

Merged
syoyo merged 30 commits intoreleasefrom
copilot/sub-pr-408-again
Mar 3, 2026
Merged

Add regression test for numpy_num_face_vertices() fix (issue #400)#418
syoyo merged 30 commits intoreleasefrom
copilot/sub-pr-408-again

Conversation

Copy link
Contributor

Copilot AI commented Mar 3, 2026

  • Understand existing test structure (tests/python/tinyobjloader_tests/)
  • Create models/issue-400-num-face-vertices.obj with quad/triangle faces
  • Add file-based regression test in test_loader.py (test_numpy_num_face_vertices_from_file) that loads the .obj file and verifies numpy_num_face_vertices() returns correct values with the right dtype
  • Fix black formatting: pyproject.toml sets line-length = 140, so wrapped np.testing.assert_array_equal calls should be on single lines

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…es()

Co-authored-by: syoyo <18676+syoyo@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix numpy_num_face_vertices() type mismatch Add regression test for numpy_num_face_vertices() fix (issue #400) Mar 3, 2026
Copilot finished work on behalf of syoyo March 3, 2026 17:40
Base automatically changed from numpy-fixes to release March 3, 2026 17:52
@syoyo
Copy link
Collaborator

syoyo commented Mar 3, 2026

@copilot do python code format to solve the code format CI failures

Co-authored-by: syoyo <18676+syoyo@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 3, 2026

@copilot do python code format to solve the code format CI failures

Fixed in f86ce63. The issue was that pyproject.toml sets line-length = 140 for black, but the wrapped np.testing.assert_array_equal(...) calls in the newly added tests fit on single lines under that limit. Reformatted accordingly.

Copilot finished work on behalf of syoyo March 3, 2026 18:18
@syoyo syoyo marked this pull request as ready for review March 3, 2026 19:32
Copilot AI review requested due to automatic review settings March 3, 2026 19:32
@syoyo syoyo merged commit d51d250 into release Mar 3, 2026
32 of 33 checks passed
@syoyo syoyo deleted the copilot/sub-pr-408-again branch March 3, 2026 19:33
Copy link
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

Adds a file-based Python regression test and corresponding OBJ fixture to ensure numpy_num_face_vertices() returns correct values (and dtype) for mixed face arity, covering issue #400.

Changes:

  • Add models/issue-400-num-face-vertices.obj test fixture with one quad and two triangles.
  • Add test_numpy_num_face_vertices_from_file to load the fixture via Loader.load() and validate numpy_num_face_vertices().
  • Minor refactor in test_loader.py (introduce MODELS_DIR; reformat expected index list).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/python/tinyobjloader_tests/test_loader.py Adds a file-based regression test for numpy_num_face_vertices() and introduces MODELS_DIR for locating model fixtures.
models/issue-400-num-face-vertices.obj Adds a minimal mixed-arity OBJ fixture for the regression test.

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

# Test: numpy_num_face_vertices() must return the same values with the correct dtype.
result = shape.mesh.numpy_num_face_vertices()
np.testing.assert_array_equal(result, expected_num_face_vertices)
assert result.dtype == np.dtype("uint32")
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The dtype assertion is hard-coded to uint32, but numpy_num_face_vertices() is meant to expose C++ unsigned int (mesh_t::num_face_vertices), which maps to NumPy's platform-dependent np.uintc / dtype('uintc'). To keep this test portable across platforms, assert against np.dtype(np.uintc) (or np.dtype('uintc')) instead of always uint32.

Suggested change
assert result.dtype == np.dtype("uint32")
assert result.dtype == np.dtype(np.uintc)

Copilot uses AI. Check for mistakes.
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.

4 participants