Skip to content

Fix missing f-string prefix in msh2obj texcoord error message#3302

Open
Nas01010101 wants to merge 1 commit into
google-deepmind:mainfrom
Nas01010101:fix/msh2obj-texcoord-error-fstring
Open

Fix missing f-string prefix in msh2obj texcoord error message#3302
Nas01010101 wants to merge 1 commit into
google-deepmind:mainfrom
Nas01010101:fix/msh2obj-texcoord-error-fstring

Conversation

@Nas01010101
Copy link
Copy Markdown

Problem

In msh2obj.py, the texcoord-count validation in Msh.create spans two string literals, but only the first has an f prefix:

if vertex_texcoords.size != 2 * ntexcoord:
    raise ValueError(
        f"Invalid number of texcoords: {vertex_texcoords.size} != "
        "2*{ntexcoord}."        # <-- missing f
    )

So the message emits the literal text 2*{ntexcoord} instead of the expected count, e.g.:

Invalid number of texcoords: 2 != 2*{ntexcoord}.

rather than the intended ... != 2*5.. The sibling vertices, normals, and faces messages interpolate correctly; only this one was missed.

Fix

Add the f prefix to the continuation literal.

Tests

Adds test_texcoord_size_mismatch_error_reports_expected_count, which feeds a .msh declaring ntexcoord=5 but only 2 texcoord floats and asserts the error reports the computed count (2 != 2*5.). It fails before this change (the message contains the literal {ntexcoord}) and passes after; the rest of msh2obj_test.py remains green.

The texcoord-count check in `Msh.create` spans two string literals, but only
the first has an `f` prefix, so the second emits the literal `2*{ntexcoord}`
instead of the expected count: `texcoords: 2 != 2*{ntexcoord}.` rather than
`texcoords: 2 != 2*5.`. The sibling vertex, normal, and face messages
interpolate correctly; only this one was missed.

Add the `f` prefix, and a regression test that feeds a `.msh` with a mismatched
texcoord count; it fails before this change and passes after.
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