Skip to content

Respect of the specification, error handling and refactoring. #2

@nbrr

Description

@nbrr

After implementing support for Sparse arrays, I have begun to work on support for Structure arrays. I believe it is mostly done but my test case contains char arrays so I need to implement this first. Since I have worked a bit on the code, I have a few comments.

My initial goal is to be able to read matrices from https://sparse.tamu.edu/, so that is where I picked test files. As I was testing I noticed that they don't seem to comply well with the mat file specification.
For example, array name's tag should indicate i8 but I have found it to be tagged as i32 in jgl009.mat, if I am not mistaken (I found it while decomposing the parsing process in order to debug, it's the commented tag3 here. This raises issue while parsing because of the check made on the type here. Note that when this check is disregarded, the name parses just fine as u8. We should rethink this type check even though it matches the specification. It should maybe be noted that Octave parses this file fine.

Second point is error handling. I have not done much error handling with Rust so far, but I feel that nom::ErrorKind::Custom(some int) isn't very clear. In the test case I just mentioned just before I have trouble finding where some assert went wrong. I don't really know whether the error handling that has been implemented so far is good or bad generally speaking, or how it could be improved.

Third comment is that I felt the code could be more organized. Maybe a module with elementary parsers such as numeric types, header, tags; a module with parsers for compound structures; having the content of lib.rs moved to a module; better naming for the two NumericData enums that exist in the library. If you are open to this I will give it more thought.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions