search

tshort / Eyeball.jl

Object and type viewer for Julia
Other
140 stars 4 forks source link

add support for MAT files #26

Closed bjarthur closed 2 years ago

bjarthur commented 2 years ago

related to https://github.com/tshort/Eyeball.jl/issues/25 since MAT files are HDF5

adds a dependency on Requires to avoid depending on MAT. i think this should be okay since in future no other deps will need to be added to support, e.g., HDF5 or any other package that defines a tree.

also, the docstring for getoptions says "Return a tuple of two arrays" when it appears to really return a vector of 2-tuples.

tshort commented 2 years ago

I'm leery of depending on Requires. I haven't used it for a long time, but I recall seeing issues related to precompilation and compile times. Maybe this isn't a problem with this type of package.

@bjarthur, do you notice any differences related to installation or time to first run?

tshort commented 2 years ago

Good point on the docstring. That changed at some point.

bjarthur commented 2 years ago

time to compile @require was made 5x faster about a year ago. given how tim timed it (see code snippet in the OP of the linked issue), i gather that there is no slow down unless the package that is @required is actually used. so IIUC, this PR should have no noticeable effect unless once is actually using MAT.

i haven't bothered to measure it myself though, and have not used it enough to even make a qualitative statement regarding speed difference. it's fast enough for my purposes in my typical use case, and supporting MAT is a huge win in my book.

how else would you support MAT, or any other structure that's not in stdlib?

tshort commented 2 years ago

Other alternatives to using Revise are:

All this said, I still might go with Revise. But, I'd prefer something more generic, so packages don't have to do this.

bjarthur commented 2 years ago

wait! matread returns a Dict, so it's actually worked all along. doh. so MAT support is fine, just not HDF5. ok, sorry, will close this.