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

MetadataOptions - Control Group Files Granularity #2458

Closed
wants to merge 1 commit into from

Conversation

dgault
Copy link
Member

@dgault dgault commented Jun 15, 2016

Adding new MetadataOptions which can be used to control the granularity of group files.
The following options are available:

  • Top level file grouping for locating files in the immediate directory (default option)
  • Recursive for exploring all sub directories for files to group
  • A max depth setting for exploring sub directories to a given depth

The new MetadataGroupFilesOptions contains an enum GroupFilesGranularity and a maxDepth value to represent these options. The getMaxDepth will return 0 if the granularity is set to TOP_LEVEL and Intger. MAX_VALUE if the granularity is set to RECURSIVE.

As these options are not yet implemented in any individual readers there is no specific testing required, only to confirm that the proposed options provide the desired functionality for future usage.

Add options to MetadataOptions enabling control of the granularity and
depth of group files.
@melissalinkert
Copy link
Member

Main question for me is how this gets used in practice (in readers, and as an API consumer). Does the value of MetadataGroupFilesOptions only matter if the reader returns true for isGroupFiles() (the current default)? Will the default value of MetadataGroupFilesOptions accommodate something like ScanR, where the metadata file is in the top-level directory, and all data files are (potentially) in a separate child directory?

I do also kind of wonder if the enum is necessary, since if I understand maxDepth would suffice to describe all three cases.

@dgault
Copy link
Member Author

dgault commented Jun 15, 2016

As for how this gets used, it really relies on readers to be aware of the options set and implement them, otherwise from API consumer point of view it wont be having any effect as it is. I had imagined that readers would only use the options in cases were isGroupFiles is true.

For your example of ScanR, my thinking was that currently the default is set to represent the top level directory, setting the options to RECURSIVE would represent the user wishes to search all child directories, and using LIMITED_DEPTH with maxDepth of 1 would represent searching in child folders only. So either of the latter 2 options would work with the last being the most efficient if you know you are only searching to a certain depth.

I used enum largely for readability, it certainly could be reduced to just using maxDepth value if that simplifies things.

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build BIOFORMATS-DEV-merge-push#508. See the console output for more details.
Possible conflicts:

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build BIOFORMATS-DEV-merge-push#509. See the console output for more details.
Possible conflicts:

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build BIOFORMATS-DEV-merge-push#510. See the console output for more details.
Possible conflicts:

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build BIOFORMATS-DEV-merge-push#511. See the console output for more details.
Possible conflicts:

@snoopycrimecop
Copy link
Member

Conflicting PR. Removed from build BIOFORMATS-DEV-merge-push#512. See the console output for more details.
Possible conflicts:

@sbesson
Copy link
Member

sbesson commented Jul 5, 2016

With #2456 being still a candidate for the upcoming milestone, I cannot see us coming back to this in time for 5.2.0. Closing to reduce noise. Note that with #2457, we might have a mechanism for specifying per-reader key/value options which one could envision passing to MetadataOptions.

@sbesson sbesson closed this Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants