Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Valid meshes do not require len(points) == len(obj:vn) #1397

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

GenevieveBuckley
Copy link

@GenevieveBuckley GenevieveBuckley commented Mar 6, 2023

Closes #1261
Closes #1377

This PR removes a check for point data consistency, since that restriction is not actually fulfilled by many valid mesh files.

meshio/src/meshio/_mesh.py

Lines 165 to 169 in 0138cc8

if len(self.point_data[key]) != len(self.points):
raise ValueError(
f"len(points) = {len(self.points)}, "
f'but len(point_data["{key}"]) = {len(self.point_data[key])}'
)

This PR also adds another test for this case.
I have used the example .obj file from #1261 (comment)

v  -0.50 -0.50 -0.50
v  -0.50 0.50 -0.50
v  0.50 0.50 -0.50
v  0.50 -0.50 -0.50
v  -0.50 -0.50 0.50
v  0.50 -0.50 0.50
v  0.50 0.50 0.50
v  -0.50 0.50 0.50

vn 0.00 0.00 -1.00
vn 0.00 0.00 1.00
vn 0.00 -1.00 0.00
vn 1.00 0.00 0.00
vn 0.00 1.00 0.00
vn -1.00 0.00 0.00

vt 1.00 0.00 0.00
vt 1.00 1.00 0.00
vt 0.00 1.00 0.00
vt 0.00 0.00 0.00

g Box

f 1/1/1 2/2/1 3/3/1
f 3/3/1 4/4/1 1/1/1
f 5/4/2 6/1/2 7/2/2
f 7/2/2 8/3/2 5/4/2
f 1/4/3 4/1/3 6/2/3
f 6/2/3 5/3/3 1/4/3
f 4/4/4 3/1/4 7/2/4
f 7/2/4 6/3/4 4/4/4
f 3/4/5 2/1/5 8/2/5
f 8/2/5 7/3/5 3/4/5
f 2/4/6 1/1/6 5/2/6
f 5/2/6 8/3/6 2/4/6

@GenevieveBuckley
Copy link
Author

More history/context: this point data consistency check was added in PR #1067.

Also in that PR 8e87d45, a pytest skip line was added to an existing test of the elephav.obj mesh.

@pytest.mark.skip("Fails point data consistency check.")

There's no description in the text of the PR, so I'm a little unclear about what problems this check for point data consistency was intended to prevent, or why the test was skipped.

So, while the test suite is passing for me, it may also be possible that I'll need to add another test (to check we really are preventing whatever bad thing we need to avoid). Please let me know if that's the case.

@GenevieveBuckley GenevieveBuckley changed the title Valid meshes do not require len(points) == len(obj:vt) Valid meshes do not require len(points) == len(obj:vn) Mar 6, 2023
@GenevieveBuckley GenevieveBuckley mentioned this pull request Mar 6, 2023
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.

[BUG] [BUG] len(points) != len(obj:vt)
1 participant