[x] Change from indextree to petgraph, we'll need it later anyway for ModeMaster dependencies
[x] Completely rework first implementation. Important points:
[x] Top-level Geometries must be parsed first so GeometryReferences to them can be linked on the fly
[x] Put in a "fullPath" or "fullName" or "geometryPath" or "geometryNode" containing the way it is referenced inside GDTF. Or should this be a method?
But the name of a geometry must be globally unique anyway, so what's the use? Let's not do it.
[x] What if a top-level Geometry is a GeometryReference? Can there be nested GeometryReference chains?
[x] Consider validation at each single step
[x] References
[x] Break_Offsets (Handling of Break Overwrite?)
[x] Figure out whether parse-function should be TryFrom method
No, because it takes any Read + Seek-argument, which is not semantically strong enough (is it zipped or not?, etc.)
[x] Add a clearer status message to README
[x] Remove GDTF files from GDTF-share, as they don’t have any license and the person creating them or even GDTF-share might have an issue with it (also delete on all old commits). Replace with a folder where the developer has to manually place fixture files (is API ready?) and smoke tests are run. Results should be manually inspectable, as well. Maybe include an automatically generated list of recommended fixtures, with file names, versions, source URL (manual and optional) and file hashes. Provide a way to assert certain results for certain files or to do snapshot testing. If we want to run specific tests, we ought to replicate ourselves the cases in the files, creating more minimal and maintainable test cases as well.
[x] Fix broken tests
[x] Replace Vec<HandledProblem> with a custom type alias
[x] Empty string for attribute RefFT should be parsed as "None" silently. This is against the DIN (the attribute doesn't have a default value and is therefore assumed required), but semantically it is clearly optional and the GDTF Builder always outputs an empty string instead of omitting the attribute. Throwing a problem every time seems unnecessary.
[x] Review GDTF Example Tests (variable names are messy)
[x] Write documentation on GDTF Example Tetsts in CONTRIBUTING.md
[x] Deterministic Geometry Deduplication
[x] Review Code: Refactor, Add Tests, ...
[x] Review parser::geometries.rs
[x] Check validity of everything, also against DIN
[x] Check things against our own principles
[x] Lint to disallow expect and unwrap outside tests?
[x] The code should clearly separate between domain logic (building Gdtf, validating it upon modifications, ...) and parsing logic. That will make it much easier later to offer a modifiable and serializable API.
[x] When building a struct from a document, we need to be lenient. Example: We wouldn't require Unique Geometry Names globally, only between children of each top-level geometry and between top-level geometries and all non-top-level-geometries. We break these duplications with UUID insertion, as we do already. However, what if someone references a Geometry, expecting to reference the one limited to the top-level geometry of the current DMXMode, but because we renamed it with a UUID, that now points to the wrong Geometry, pointing outside the geometry of the DMXMode, breaking that reference. How do we deal with this? Special casing UUID-renamed Geometries during parsing? We could keep a list of renames during the parsing...
[x] So basically, when we later resolve Geometry references, we first look normally, then check if it is a descendant of the right top-level geomery (we have to do that anway), if yes alright, if no look in a list of parsing renames: (top-level geometry, old_name, new_name). If old_name occurs multiple times for the same top-level geometry, error the Geometry reference. Otherwise, resolve the reference to the geometry with new_name.
[x] But when building a Gdtf struct via the API or serializing it, we want the strict definition of "Unique Geometry Names" to be obeyed.
[x] Update Deps
[x] Remove TODOs
[x] Fix Clippy Warnings about Panics in code
[x] Format all files and enable "format on save"
[x] Move hash_gdtf into main library (own module). Can we replace all that logic with the CRC-32 inside the zip? -> almost, still need to combine filename and CRC32 for each file and hash that (but it's way smaller) (benchmark that it's actually faster)
[x] Add Docstrings to functions with some explanations, this will make it easier to later remember what a function does
[x] Warn on Array Indexing, since it might panic. Is there a way to write error handling code with get that does not result in massive code blocks of Problem::Unexpected?
[x] Write a really simple benchmark against gdtf_parser, just to check we're not magnitudes slower already ;)
It's an acceptable factor 1.5 (see 645dc70)
Interestingly, with the release build opengdtf is (at this point, we barely do any work) still slightly faster:
$ cargo run --release --bin comparison_against_gdtf_parser
Finished release [optimized] target(s) in 0.06s
Running `target/release/comparison_against_gdtf_parser`
parsing 156 example files with both crates and measuring time...
gdtf-parser successfully parsed 155 fixtures
opengdtf successfully parsed 155 fixtures
gdtf-parser took 0.48860416 s
opengdtf took 0.44242147 s
opengdtf relative time to gdtf-parser: 0.9054803181142704
Firionus
commented
1 year ago
parse-function should beTryFrommethodRead + Seek-argument, which is not semantically strong enough (is it zipped or not?, etc.)Vec<HandledProblem>with a custom type aliasgetthat does not result in massive code blocks ofProblem::Unexpected?gdtf.nameof typeName