-
Notifications
You must be signed in to change notification settings - Fork 53
New types of datasets supported for Delmic HDF5 format #328
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
Merged
Merged
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
fb51467
New types of datasets supported for Delmic HDF5 format
noemiebonnet 1160543
Improve documentation message. Change header to standard RosettaSciIO…
noemiebonnet 84bc2aa
add point measurements handling
noemiebonnet f528b1b
Add raw metadata
noemiebonnet 5d937bd
add lumispy condition in metadata
noemiebonnet 80bfe2c
Add testing functions for CL, SE, survey signals
noemiebonnet 670bbc5
Create test functions for data stacks
noemiebonnet 597fa3a
Add new metadata signal_types and corresponding test functions
noemiebonnet 1c68f03
Add test functions and reference files for single-point measurements
noemiebonnet 3142543
Add test function and file for streak camera spot measurement
noemiebonnet ebae3a1
Add documentation
noemiebonnet a5d32a6
delmic format: code refactoring and small feature improvements
pieleric 72d2469
Add changelog entry for new Delmic format
pieleric 5db37c7
delmic format: minor clean-ups
pieleric 70bb83d
delmic: adjust signal parameter to default to "cl"
pieleric 106687e
delmic: refactor tests to be a single test per type of data loading
pieleric 5afc7b2
delmic: sort datasets by type
pieleric 6a80016
delmic: don't count coverage on exception handler
pieleric 9bac123
doc delmic: minor improvements
pieleric e6e1985
delmic: move survey to be very last dataset
pieleric File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were discussing what is actually the best default here - reading only CL to return a single signal item as in most other readers or the list with all items, but I guess the main point would be that it is well documented.
Also, I am not certain having the survey first is the best order of the signals in the list. I assume the order is motivated by the odemis files? Generally, I would find it more intuitive to have the actual CL signal first, then concurrent and last survey (making the survey always last item, even if there are multiple streams in one file).
Finally, I would say there is not really a point in reading in the concurrent
sewhen it is dimensionless (for spectra, single transients or streak images).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In particular having the
anchorregion data from drift correction included in the default loading seems a bit overload - so maybe default tocland have an optionallto include the other datasets?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I've now changed it to return only the
"cl"datasets by default. Also changed fromNoneto"all"to get all the datasets.Concerning the datasets with a single point, although it's probably not useful, I'd rather always return them when that type of dataset (or "all") is requested. This keeps the code simple and makes sure that if for some reason the user actually do care about this single pixel, they can still read it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough for the datasets with a single point. Only, I would still tend to reverse the order of the different signals in the list for "all" to have the 'cl' first (as it is the main one and now also the one that is read by default) and then add the additional signals with 'survey' last.