BF: OBJ, legacy VTK, VTK XML and Nastran correctness - #46
Merged
Conversation
P1.1 and P1.2 of the parity plan. Face indices now mean what the format says they mean. A negative index counts back from what has been declared so far instead of indexing a numpy array from the end, and an index naming a record the file does not have raises CodecError naming the line instead of quietly putting a different vertex in the mesh. vt records are read at last. OBJ indexes vt and vn per face corner, so a file may hold more of either than it holds vertices: each corner assigns to its vertex, a vertex given two different values keeps the last and warns that a per-vertex array cannot hold a seam, and records no face indexes are kept only when there is exactly one per vertex. Both are written back as vt/vn with the corner naming its own vertex, so texture coordinates and normals survive a round trip. Groups: a face in no group is now written after a bare g, which stops it inheriting the group of the face above it, and a bare g on read clears the active groups instead of inventing a 'default' tag that was never in the mesh.
P1.3 and P1.4 of the parity plan. COLOR_SCALARS and NORMALS were unknown keywords to the unstructured and polydata attribute scans, and an unknown keyword ends the scan: the array and everything after it was dropped without a word. COLOR_SCALARS is the one section whose type its line does not name - one unsigned char per component in binary, a float in ASCII - so it cannot share the SCALARS branch. The legacy structured datasets expand their header into an explicit point array, which threw the grid away. DIMENSIONS, ORIGIN and SPACING now land in global_attrs as vtk_dimensions / vtk_origin / vtk_spacing for STRUCTURED_POINTS, and DIMENSIONS for STRUCTURED_GRID and RECTILINEAR_GRID. Four neighbouring cases turn out not to be bugs here: a CRLF header, a padded POINTS keyword and float32 points all read already, and POLYDATA already covers VERTICES, LINES, POLYGONS and TRIANGLE_STRIPS. Each keeps a test, so none of them can regress unnoticed.
P1.5 of the parity plan. The XML writers declared version="0.1" in the VTKFile header, a version no VTK release defines; they now say 1.0. Files declaring 0.1 still read, since that is what everything written before today says. A DataArray polyxios cannot decode - type="String", which holds labels rather than numbers, or any type it does not know - was returned as an empty array and dropped by the caller without a word, so a file quietly came back missing a field. It is now skipped with a warning naming the array and its type, and the arrays around it are read as before. The rest of the item turns out to be immune: two Pieces are both read and the second one's cells index the second one's points, a file with no points reads to an empty PolyData rather than crashing, and vertex order is byte-stable through a round trip. Each keeps a test.
skoudoro
force-pushed
the
p1/obj-correctness
branch
3 times, most recently
from
August 20, 2026 15:00
f8040c1 to
a3d7a0e
Compare
P1.6 of the parity plan, plus the review pass over P1.3 to P1.5. A Nastran real field now uses whichever legal spelling fits. Bulk data lets the exponent's E go when its sign is written (1.234-10 is 1.234E-10), and nothing reads +07 differently from +7, so both shorthands are offered when the explicit form does not fit; that is three columns back, three more significant digits in an eight-column field. The other half is rounding. %G rounds to nearest, and at the top of the double range rounding up steps off it: the largest double shortens to 1.8E+308, which reads back as infinity, and the search used to fall through every precision and refuse the value. The mantissa is now also offered one unit in the last place toward zero, borrowing across the decimal point, so 1.79+308 goes out instead of an error. Candidates are ordered by what they read back as rather than by how many digits they carry. Spending precision one digit at a time and taking the first form that fit let a stepped-down mantissa win a field an exact form two columns shorter would also have fit: 1e7 went out as 9999999. where 1.E+07 was available, and 1e15 as 999999999999999. in a large field. Exact spellings are swept for first, at every precision, and only a value no field can hold exactly falls through to the closest one. Swept against the old search over 17k values across both widths: nothing got worse, 1064 got better. OBJ leaves out a vn or vt attribute that does not hold one row per vertex. Only the column count was checked, so an attribute with fewer rows than the mesh wrote faces indexing vt records that were not in the file - a three-vertex mesh carrying one texture coordinate wrote a file this reader answered with CodecError. A one-dimensional attribute is read as one value per vertex rather than as a single row. Legacy VTK reads TEXTURE_COORDINATES and steps over a LOOKUP_TABLE definition. Those were the last two attribute keywords the format defines that the scan did not know, and in a binary file an unknown keyword ends the scan, so a file carrying either lost every array after it - fran_cut.vtk in the corpus is one. A keyword outside the set now warns about what it costs rather than truncating in silence, NORMALS and COLOR_SCALARS are read from the three structured datasets that scan their attributes themselves, and a binary attribute running past the end of the file raises CodecError naming the array instead of failing in a reshape that named nothing. A .vtu or .vtp Piece missing its Points element altogether is refused the way a short one is, and the offset later pieces are shifted by is counted before the piece is added rather than after. An attribute the pieces shape differently - one calling it scalar, the next a vector - is dropped with its shapes named rather than raising a bare ValueError out of concatenate. The property tests sweep 1e-30 to 1e30, the denormals and both ends of the double range through both field widths: nothing raises, nothing overruns its field, and everything reads back finite with its sign. Every power of ten reads back exactly. Across the 229-file corpus the only read that changed is fran_cut.vtk gaining its texture coordinates, and 80 files written back through five formats are byte-identical. Review pass on the above. The exact-spelling sweep cost more than it was worth. It asked every precision from seventeen digits down, spelling and parsing candidates at each, and a large-field write paid about a hundred parses per coordinate. Rounding to fewer significant digits than repr carries cannot read back as the value whatever form it is spelled in, so the sweep now floors there; neither sweep spells a candidate at a precision the field could not hold in the first place, and the stepped mantissas - the expensive half - are built only once the rounded ones have all missed. Twenty thousand GRID* cards went from 10.3s to 1.0s. Swept over 1,079,901 comparisons across nine field widths against the old search: every value is written exactly as before. The three structured VTK readers dropped their CELL_DATA. Each walked its own attributes through a chain that asked only about points, so every cell array fell past it in silence - the cell scalars in VTK's own SampleStructGrid.vtk among them. The three now share one scanner, which also gives them TEXTURE_COORDINATES and TENSORS and checks a binary block against the end of the file the way the shared parser already did. An array whose declared length matches neither the points nor the cells is dropped with a warning rather than reaching PolyData as a validation error about lengths. A structured grid extends along whichever axes it declares. DIMENSIONS 3 1 3 was read as two lines over the first two points instead of four quads over all nine, and a column along y or z was indexed with the stride of a row along x; only grids flat in z and rows along x came out right. An unknown keyword in an ASCII legacy VTK file now warns the way the binary scan already did, an attribute section whose values run into the next header raises CodecError naming the array and the line rather than a bare ValueError out of float(), and a keyword one of the structured readers does not handle says so - skipping the line does not step over the payload, so in a binary file the scan carries on inside it. A .vtu or .vtp Points array whose size is not a whole number of tuples per point names the Piece; only a short array was caught, so ten values for three points reached reshape. The offset later pieces are shifted by is carried along rather than re-summed per piece. A v, vn or vt record that does not carry what its directive needs, or carries something that is not a number, names the line instead of reaching the caller as a bare IndexError or ValueError. A vt carrying a third component keeps the two a surface uses rather than making the records ragged. Writing an attribute that holds no numbers drops it with a warning instead of raising out of asarray, and reading no longer names the source once per line. Across the 59-file legacy VTK corpus the only read that changed is SampleStructGrid.vtk gaining the cell array it used to drop. test_a_power_of_ten_is_written_exactly computed its powers with **, and pow is not correctly rounded everywhere: glibc and MSVC answer 10.0 ** 23 with the double one unit above 1e23, macOS with 1e23 itself. That neighbour needs seventeen significant digits - twenty-one characters in the shortest spelling the format has - so no eight or sixteen column field can hold it, and the test was asking the writer for a field wider than bulk data defines. It failed on Linux and Windows and passed on macOS. The powers are spelled as literals now. The writer is unchanged: 1.E+23 is the closest a field of either width can come to that neighbour, and it is what both the old search and the new one return for it.
skoudoro
force-pushed
the
p1/obj-correctness
branch
from
August 20, 2026 15:07
a3d7a0e to
a247ab8
Compare
Legacy .vtk files written by VTK 9 could not be read at all. The two numbers on a v5.1 CELLS line are the length of the OFFSETS array and the length of CONNECTIVITY, not the cell count; reading the first as a cell count ran the offsets into the CONNECTIVITY keyword. The offsets are now counted up to that keyword, so files spelling the line either way read. POLYDATA gained the layout altogether - its POLYGONS, LINES, VERTICES and TRIANGLE_STRIPS sections knew only the v4.2 form. Writing v5.1 declared the cell count there, which VTK's own reader takes literally: it stopped with "Error reading cell array connectivity header" and returned a mesh with no cells. Files polyxios wrote before still read. A METADATA block - written after every array since VTK 4.2, text even in a binary file - is stepped over. In ASCII it warned twice about keywords it named as dropped; in binary it ended the attribute scan; inside a FIELD block it was read as an array header and took the next array with it. A binary STRUCTURED_GRID stepped the line cursor past the POINTS payload and then once more over the newline that ended it, skipping whichever section came next. A RECTILINEAR_GRID now follows its coordinate arrays rather than a DIMENSIONS header that disagrees with them, which generated cells indexing points that do not exist. .vti, .vts and .vtr honour NumberOfComponents on read, and .vtr declares it on write and writes each array in the type its DataArray names - an Int32 attribute was cast to float64 and read back as the bit pattern of a double. A .vtu or .vtp Points array of a type holding no numbers names the type. Warnings blame the caller rather than this module, and stepping past a binary payload is a binary search over the line offsets.
A .vtr attribute of a dtype no VTK type names fell back to a Float64 header but kept the dtype's own bytes, so a boolean mask went out as one byte per value under a header claiming eight. Every XML writer declared the second dimension of an array as its component count, so an (n, 3, 3) tensor came back nine times too long. Both tensor branches of the legacy writer spelled their numbers into binary files. The ASCII writers spelled to ten significant digits under a header declaring double, losing the last seven of every coordinate. Also: a METADATA block left open now ends at the geometry keyword after it rather than swallowing the rest of the file; a malformed attribute section costs the section rather than the mesh it was read after; .vti/.vts/.vtr drop an attribute whose rows match no mesh, as .vtu and .vtp already did; a failing Piece is named by its index; an OBJ vertex conflict is seen in whichever component it is in. Structured hexahedra are built with array arithmetic rather than a loop per cell, the legacy readers share one line splitter, and the OBJ per-vertex fold is one pass over the corners.
A legacy STRUCTURED_GRID whose DIMENSIONS does not cover its POINTS array now hands the points back without cells and warns about the two counts. The cells are strides through the layout the header describes, so a header naming more points than the file delivered built connectivity indexing points that are not there and the read returned a PolyData that fails validate. RECTILINEAR_GRID already reconciled the two. An attribute section is read by the count its own header declares rather than by the mesh's. They agree in a well-formed file, and where they do not the section's is the only number that says where one array ends and the next begins - reading by the mesh's walked an array straight into the header after it. An array covering no point or cell is then dropped with a warning, as the structured readers already did. A binary SCALARS section without a LOOKUP_TABLE line - which the format leaves optional - reads its own values. The line was consumed unconditionally, swallowing payload up to its first 0x0a byte, and a payload holding none rewound the scan to the top of the file. A binary POINTS, CELLS or CELL_TYPES block is bounds-checked before it is sliced, the way the attribute blocks already were. The whole-file bound the header check applies clears a block that still runs off the end, and the reshape after the short slice named neither the array nor the file. A header missing a field, or spelling a count as something that is not a number, names the file and the line - the byte offset, in a binary file. SCALARS with no array name reached the caller as a bare IndexError and SCALARS s float x as a bare ValueError, in all three scans. .vti, .vts, .vtp and .vtu write each attribute in the type the array is held in, as .vtr does, and the ASCII body of a DataArray is parsed into the type the element declares rather than through float() first. Cast to a double at either end, an int64 identifier past 2**53 came back a different number. Reading an OBJ file resolves a face corner inline when it names a plain index inside what has been declared; the rest still go through _resolve_index, where the message naming the line lives. A record carrying exactly the components its directive spells skips the padding and the slice that feeds it. A float32 attribute is spelled at its own width in an ASCII DataArray rather than widened to a double first.
Legacy .vtk binary blocks are read as the type their header names. POINTS was resolved with a double-or-float test, so 'POINTS n int' came back as coordinates the file never held, and a type name with no numpy equivalent was guessed at. One helper now resolves every binary header and raises CodecError naming the type it cannot read; the names VTK writes for 64-bit and signed-char arrays are added to the table. ASCII payloads are unaffected - their values are text whatever the header calls them. Geometry and section headers that spell a count as something that is not a number, or leave it out, name the file and the line: POINTS, CELLS, CELL_TYPES, POINT_DATA, CELL_DATA, DIMENSIONS, ORIGIN, SPACING and the coordinate arrays reached the caller as a bare ValueError or IndexError. A CELL_DATA array in a STRUCTURED_GRID is measured against the cells the mesh ends up with, not the cells DIMENSIONS describes: a header its POINTS array does not cover leaves no cells, and the array was kept against the header's count, so the read returned a PolyData that fails validate. OBJ: a face index spelled with a superscript digit raises CodecError - str.isdigit admits it and int() then refuses - a bare mtllib or o names nothing rather than the empty string, and a material attribute that does not cover the faces is dropped with a warning instead of running off the end mid-write. XML: a Piece NumberOfPoints that is not a count, an Extent that is not six whole numbers, and a NumberOfComponents that is not a count are all named rather than raising out of int() or an unpack. A Nastran real field below one drops its leading zero when that column is a significant digit, since bulk data reads '.5' as '0.5'.
A legacy .vtk header spelling DATASET FIELD was refused: the dispatch asked what the line starts with while it still carried its DATASET keyword, so the branch never ran. A v5.1 CELLS section is now found by what follows the header rather than by the version in the first line, which compared as a string and sorted 10.0 below 5.1, and a METADATA block between its offsets and its connectivity - which every VTK release since 9.0 writes - is stepped over instead of read as offsets. A .vti, .vts or .vtr extent flat along an axis is a sheet of quads, and a run of lines when it is flat along two. All three read it as a grid of no cells, which left every CellData array belonging to nothing. An ASCII DataArray holding a value its declared type is too narrow for wraps with a warning rather than escaping as a bare OverflowError, and an attribute of a kind no DataArray can hold names itself instead of dying in a conversion. The warnings these codecs raise are blamed on the code that asked for the file; every one pointed a frame short.
skoudoro
commented
Aug 21, 2026
skoudoro
left a comment
skoudoro
left a comment
Collaborator
Author
There was a problem hiding this comment.
ok, it took time and many iteration but it seems ok. merging
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OBJ
CodecErrornaming the line instead of silently landing on a different vertex.vtrecords are read.vt/vnare indexed per face corner, so each corner assigns to its vertex; a vertex given two values keeps the last and warns, and unindexed records are kept only when there is exactly one per vertex. Both are written back, so texture coordinates and normals survive a round trip.grather than inheriting the group above it; a baregon read clears the active groups instead of inventing adefaulttag.Legacy VTK
COLOR_SCALARSandNORMALSare read. They were unknown keywords, and an unknown keyword ended the attribute scan — the array and everything after it was dropped without a word.DIMENSIONS,ORIGINandSPACINGland inglobal_attrsasvtk_dimensions/vtk_origin/vtk_spacinginstead of being thrown away when the header expands into an explicit point array.VTK XML
version="1.0"instead of0.1, a version no VTK release defines. Files declaring0.1still read.DataArraythat cannot be decoded (type="String", or any unknown type) is skipped with a warning naming the array and its type, rather than returning empty and vanishing from the result.Nastran
1.234-10), and the mantissa may be stepped one ULP toward zero so values at the top of the double range go out instead of raising.