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

Add xio_gdal_handler #133

Merged
merged 3 commits into from
Jan 13, 2021
Merged

Conversation

davidbrochart
Copy link
Member

@davidbrochart davidbrochart commented Dec 24, 2020

In addition to stream-like objects, this PR gives support for FILE-like objects to format handlers.
This first commit only changes the xio_binary format. I'll update the other formats after review.
See #132.

@davidbrochart
Copy link
Member Author

davidbrochart commented Dec 26, 2020

Now the file and vsilfile classes are pretty close to a stream. I think we should just have a "wrapper" class for file-like objects and stream-like objects, that should implement these common methods:

  • read(std::string& buffer)
  • write(std::string& buffer, std::size_t size)

And get rid of the "overload" dump_bin_imp and load_bin_imp functions.

@davidbrochart
Copy link
Member Author

The stream and file wrapper classes implement the methods needed in the compression formats that we currently have, so they are compatible with a stream or a file type.
If a future compression format needs a proper stream or file object (not our wrapper), we could specialize the load/dump functions for this particular type (e.g. stream) and get the object from our wrapper, and throw an "unsupported format" for the other type (e.g. file), like "GDAL doesn't support this format (because GDAL is a file-based IO handler, and the format is stream-based)".
This is the best I can think of, since wrapping a file into a stream doesn't seem to be a trivial task.

@davidbrochart davidbrochart changed the title [WIP] Add file-like support in format handlers Add xio_gdal_handler Dec 26, 2020
@davidbrochart
Copy link
Member Author

Not sure about the name xio_gdal_handler. GDAL's documentation talks about Virtual File System, and the functions are called VSI functions. But neither xio_vfs_handler or xio_vsi_handler sound good to me.

@davidbrochart davidbrochart force-pushed the file_support branch 3 times, most recently from 5f0fd98 to 53903c8 Compare December 28, 2020 19:13
@JohanMabille
Copy link
Member

JohanMabille commented Jan 13, 2021

LGTM!

I could be nice to do a pass on API homogeneity, but this can be done in a dedicated PR!

@JohanMabille JohanMabille merged commit 9820cc4 into xtensor-stack:master Jan 13, 2021
@davidbrochart davidbrochart deleted the file_support branch January 13, 2021 12:46
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