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

feat: skip fields with undocumented classes instead of hard error #119

Closed
wants to merge 2 commits into from

Conversation

mcmcgrath13
Copy link

Currently, if a .mat file contains fields such as audioplayer and others that are undocumented, matread will error without returning any read data. This pull request adds functionality to warn the user and return missing instead. The functionality is implemented similarly in both MAT_V5 and MAT_HDF5, the main difference I've noted is that these undocumented fields sometimes come with a ghost field at the root level which is handled slightly differently.

In my work, I am trying to process .mat files with many fields/structures/sub-structures and often these undocumented fields are not the ones I am trying to work with and it is frustrating when the read process crashes (and in V5 I can't work around this by reading in only sub-paths of the file). This PR will allow working with the rest of the file and skip only what is unreadable.

This is related to #23 , but instead takes the approach of skipping these fields. If #23 is ever completed/incorporated, this should become obsolete functionality.

@mcmcgrath13
Copy link
Author

mcmcgrath13 commented Aug 9, 2019

This may actually be redundant to #118 for v6/v7 files. It reads in the data, it doesn't make any sense to me based on what I see in MatLab, but it doesn't crash, so that works for me.

The HDF5 may still need this to not fatally error on these classes.

@mcmcgrath13
Copy link
Author

@yuyichao @timholy how would you like to proceed with this and #118? Thanks!

@timholy
Copy link
Member

timholy commented Sep 21, 2019

Merged #118. Thanks though!

@timholy timholy closed this Sep 21, 2019
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.

2 participants