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

Validate C3DFile before writing #20

Merged
merged 49 commits into from
Aug 12, 2024
Merged

Validate C3DFile before writing #20

merged 49 commits into from
Aug 12, 2024

Conversation

halleysfifthinc
Copy link
Owner

Previously, the validate function modified groups and parameters to be consistent/correct for creating the Julia C3DFile object. This is problematic where certain Julia requirements are stricter than the C3D file spec (e.g. labels need not be unique, but Dict keys must be) and the validation resulted in loss of the as-read data.

This PR generally cleans things up and reduces differences between the original .c3d and the .c3d written by C3D.jl.

- Move label uniqueness and deduplication checks from validation stage to
C3DFile constructor.
- Add warning when duplicate labels are renamed
- Add immediate validation to verify group uniqueness
- Add validation to enforce parameter uniqueness
    - Needed `==` and `hash` functions for `Parameter`
    - Required adding a kwarg to `readc3d` (explained in docstring)
- Validation test needed update due to above changes
(Any added groups or parameters are required by the spec and/or to read
properly, therefore the original group/parameter being missing is
incorrect and the "difference" of the added group/parameter is
correct/more proper)
… when relevant

(The parameter write function is only designed for allowed scalar types
and is incorrect otherwise)
@halleysfifthinc halleysfifthinc merged commit 07439a8 into master Aug 12, 2024
3 checks passed
@halleysfifthinc halleysfifthinc deleted the write-validation branch August 12, 2024 23:55
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