Force n_index to be parsed when empty string

[Liam-Deacon]

I have an .obj file provided to me, but it cannot parse it as I get a traceback stating:

~/.local/lib/python3.7/site-packages/pywavefront/obj.py in consume_faces(self, collected_faces)
    383                 except ValueError:
    384                     t_index = 0
--> 385                 n_index = (int(parts[2]) - 1) if has_vn else None
    386 
    387                 # Resolve negative index lookups

ValueError: invalid literal for int() with base 10: ''

My solution was to wrap the assignment for n_index in a try-except block as was done for t_index, which allowed the file to be successfully parsed.

[einarf]

Do you have the face data triggering this issue? Every time we fix a bug it's important to add a unit test for this specific condition.

[Liam-Deacon]

Hi @einarf - sorry the data is proprietary, so I can't share it I'm afraid. To make matters worse I am just a dumb user of the data in the sense that don't have any of the software used in creating and exporting the problematic model... all of which complicates things!

I definitely understand the desire to have unit tests for new/changed functionality or edge cases - how did you generate the test obj files? I could try to replicate the problem using a dummy obj and then add a test for it.

[einarf]

Test files are created by hand. Just a few lines are needed. The format of the f-line that triggers this error is especially important.

I'm assuming this is a new case were the normal index is not specified:

f 6/7/ 5/6/ 7/5/

You could simply search up lines containing / (slash + space) to verify this.

[einarf]

That does seem to be the case after adding a new test file, but it would be nice is you could confirm this.

>               n_index = (int(parts[2]) - 1) if has_vn else None
E               ValueError: invalid literal for int() with base 10: ''

[einarf]

I'll add test case for this. If you could confirm that you indeed have f-lines with unspecified normal index, empty or non-numeric value we at least know the test is sufficent!

[einarf]

1.3.2 is now on PyPI : https://pypi.org/project/PyWavefront/1.3.2/

If you don't see any evidence of empty or non-numeric normals in faces, please poke asap 😃

Thanks for raising the issue and providing a PR!

[Liam-Deacon]

Many thanks for your help @einarf 👍

Just as a follow up I grepped the lines in the obj file beginning with f and got a lot of lines like the following:

f 275// 274// 273// 272// 271// 270// 269// 268// 267// 266//
f 258// 257// 256// 255// 254// 253// 252// 261// 260// 259//
f 4766// 2186// 2185// 4767//

Does this show the empty normals you speak of?

The data also contains a number of entries that follow the f-line format in your earlier example:

f 7640/1/ 7643/1/ 7641/1/
f 7652/1/ 7653/1/ 7654/1/ 7655/1/ 7656/1/ 7657/1/ 7658/1/ 7659/1/ 7660/1/ 7661/1/ 7662/1/ 7663/1/ 7664/1/ 7665/1/
f 7666/1/ 7667/1/ 7668/1/ 7669/1/

Hope this helps settle this one - and thanks again for all of your help :-)

[einarf]

Yup. Looks like that closes the case completely and we have tests covering these cases.

Thanks again!