Read zipped input data #391
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
alexdewar
force-pushed
the
read_zipped_input_data
branch
from
20a246f to
c8dd65a
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
jamesturner246
approved these changes
May 30, 2024
alexdewar
force-pushed
the
read_zipped_input_data
branch
from
c8dd65a to
fb9e4e2
Compare
alexdewar
force-pushed
the
read_zipped_input_data
branch
from
fb9e4e2 to
c836cbe
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
dalonsoa
requested changes
May 30, 2024
Comment on lines
+40
to
+49
| if (!std::filesystem::create_directories(out_path)) { | ||
| throw std::runtime_error{ | ||
| fmt::format("Failed to create directory: {}", out_path.string())}; | ||
| } | ||
| } else { | ||
| std::ofstream ofs{out_path}; | ||
| if (!ofs) { | ||
| throw std::runtime_error{ | ||
| fmt::format("Failed to create file: {}", out_path.string())}; | ||
| } |
There was a problem hiding this comment.
Would it be useful to attach the original error to get an idea of why these fail, so the user can investigate?
There was a problem hiding this comment.
The only reason it'll fail here is because of an IO error and C++ won't give any information beyond that, unfortunately.
This was referenced May 30, 2024
Suggested by @dalonsoa.
|
clang-tidy review says "All clean, LGTM! 👍" |
dalonsoa
approved these changes
May 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the option to provide the input data as a path to a zip file rather than to a directory, e.g.:
Note that the
index.jsonfile must be in the root folder of the zip file otherwise it won't work.While this feature isn't particularly useful by itself, it's a necessary step to being able to automatically download input data from a URL (#364).
I haven't added tests yet because the input files are likely to be moved out of this repo soon (#366), so I'll wait until we know what the final project structure looks like first. I didn't want to commit a zip file to the repo either if its contents are likely to change soon.
I've also broken out the schema-related code into its own
.cppfile asdatamanager.cppwas getting a bit bloated.Closes #363.