fulcrumgenomics / dagr

A scala based DSL and framework for writing and executing bioinformatics pipelines as Directed Acyclic GRaphs
MIT License
69 stars 14 forks source link

Allow building a Core resource from String #347

Closed clintval closed 5 years ago

clintval commented 5 years ago

I have a pipeline where I would like to parameterize the compute resources for one of the tasks using the CLI like this toy example:

@clp(description = "Demo!", group = classOf[Pipelines])
class ExamplePipeline(
  @arg(doc="The cores for task 1") val cores: Cores = Cores(6.0),
  @arg(doc="The memory for task 1") val memory: Memory = "2G"
) extends Pipeline {

  override def build(): Unit = {
    task1 = new ShellCommand("echo", "task1").requires(cores, memory)
    task2 = new ShellCommand("echo", "task2")
    root ==> (task1 :: task2)
  }
}

At runtime I get:

...
--cores=Cores                 The cores for task 1. [Default: 6.0].
--memory=Memory               The memory for task 1. [Default: 2G].

Argument 'cores' could not be constructed from string
    ...Could not build a 'Cores' from a string: 6: dagr.core.execsystem.Cores$.apply(java.lang.String)
java.lang.Class.getMethod(Class.java:1786)
com.fulcrumgenomics.commons.reflect.ReflectionUtil$$anonfun$$nestedInanonfun$buildUnitFromString$1$1.$anonfun$applyOrElse$1(ReflectionUtil.scala:407)
scala.util.Try$.apply(Try.scala:209)
com.fulcrumgenomics.commons.reflect.ReflectionUtil$$anonfun$$nestedInanonfun$buildUnitFromString$1$1.applyOrElse(ReflectionUtil.scala:401)
com.fulcrumgenomics.commons.reflect.ReflectionUtil$$anonfun$$nestedInanonfun$buildUnitFromString$1$1.applyOrElse(ReflectionUtil.scala:399)
scala.runtime.AbstractPartialFunction.apply(AbstractPartialFunction.scala:34)
scala.util.Failure.recover(Try.scala:230)
...

Would you be willing to allow for building Core resources from Strings?

EDIT: Fixed type of cores in toy example.

codecov-io commented 5 years ago

Codecov Report

Merging #347 into master will increase coverage by 0.09%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #347      +/-   ##
==========================================
+ Coverage   91.44%   91.53%   +0.09%     
==========================================
  Files          31       31              
  Lines        1145     1146       +1     
  Branches      115      108       -7     
==========================================
+ Hits         1047     1049       +2     
+ Misses         98       97       -1
Impacted Files Coverage Δ
...src/main/scala/dagr/core/execsystem/Resource.scala 95.91% <100%> (+2.16%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 324f219...d4f8654. Read the comment docs.

nh13 commented 5 years ago

@clintval in your example, I think val cores: Cores = 6.0 will still need to be val cores: Cores = Cores(6.0) as there's no implicit in scope to convert type Double to type Cores, but other than that, LGTM.