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

Clarifying ADAMSaveAnyArgs fields #1295

Open
ryan-williams opened this issue Nov 30, 2016 · 4 comments
Open

Clarifying ADAMSaveAnyArgs fields #1295

ryan-williams opened this issue Nov 30, 2016 · 4 comments

Comments

@ryan-williams
Copy link
Member

At the very least, one or both of these should probably be renamed? (code):

  /**
   * If true and saving as a legacy format, we will write shards so that they
   * can be merged into a single file.
   *
   * @see deferMerging
   */
  var asSingleFile: Boolean

  /**
   * If true and asSingleFile is true, we will not merge the shards once we
   * write them, and will leave them for the user to merge later. If false and
   * asSingleFile is true, then we will merge the shards on write. If
   * asSingleFile is false, this is ignored.
   *
   * @see asSingleFile
   */
  var deferMerging: Boolean

It seems like asSingleFile now really means "written such that each shard can be merged, i.e. BAM header is not prepended on each partition, only the first one", and deferMerging means "leave the unmerged, headerless partitions in place"?

There are actually 3 possibilities, right? Would an enum be clearer?

@fnothaft
Copy link
Member

fnothaft commented Dec 1, 2016

There are actually 3 possibilities, right? Would an enum be clearer?

I would agree with that. An Enum would SGTM!

@heuermh
Copy link
Member

heuermh commented Dec 7, 2016

I'd be ok if ADAMSaveAnyArgs went away and the enum moved to SaveArgs in utils:

https://github.com/bigdatagenomics/utils/blob/master/utils-cli/src/main/scala/org/bdgenomics/utils/cli/ParquetArgs.scala#L30

sortFastqOutput also feels misplaced.

@heuermh
Copy link
Member

heuermh commented Dec 7, 2016

On further consideration, SaveArgs, LoadFileArgs, SaveFileArgs, and ParquetLoadSaveArgs are all rather worthless, since we always redefine INPUT and OUTPUT meta variables so that we can be more specific about supported file types in the description.

@heuermh
Copy link
Member

heuermh commented Oct 1, 2019

Note there is also disableFastConcat now

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

No branches or pull requests

3 participants