viash-io · rcannood · Dec 12, 2023 · Oct 19, 2023 · Oct 19, 2023 · Oct 20, 2023
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,8 @@ TODO add summary
   Additionally, the `native platform` and `docker platform` became a `executable runner`, `nextflow platform` became a `nextflow runner`.
   The fields of `docker platform` is split between `docker engine` and `docker runner`: `port`, `workdir`, `setup_strategy`, and `run_args` are captured by the `runner` as they define how the component is run. The other fields are captured by the `engine` as they define the environment in which the component is run. One exception is `chown` which is rarely set to false and is now always enabled.
 
+* `arguments`: Merge arguments into argument_groups during a json decode prepare step (PR #574). The `--parse_argument_groups` option from `ns list` and `config view` is deprecated as it is now always enabled.
+
 ## NEW FUNCTIONALITY
 
 * `export json_schema`: Add a `--strict` option to output a subset of the schema representing the internal structure of the Viash config (PR #564).

diff --git a/src/main/scala/io/viash/Main.scala b/src/main/scala/io/viash/Main.scala
@@ -290,6 +290,9 @@ object Main extends Logging {
         val errors = testResults.map(_._1).flatMap(_.status).count(_.isError)
         if (errors > 0) 1 else 0
       case List(cli.namespace, cli.namespace.list) =>
+        if (cli.namespace.list.parse_argument_groups()) {
+          info("Warning: --parse-argument-groups is deprecated and effectively always enabled.")
+        }
         val configs = readConfigs(
           cli.namespace.list,
           project = proj1,
@@ -300,8 +303,7 @@ object Main extends Logging {
         val configs2 = namespaceDependencies(configs, None, proj1.rootDir)
         ViashNamespace.list(
           configs = configs2,
-          format = cli.namespace.list.format(),
-          parseArgumentGroups = cli.namespace.list.parse_argument_groups()
+          format = cli.namespace.list.format()
         )
         val errors = configs.flatMap(_.status).count(_.isError)
         if (errors > 0) 1 else 0
@@ -321,6 +323,9 @@ object Main extends Logging {
         val errors = configs.flatMap(_.status).count(_.isError)
         if (errors > 0) 1 else 0
       case List(cli.config, cli.config.view) =>
+        if (cli.config.view.parse_argument_groups()) {
+          info("Warning: --parse-argument-groups is deprecated and effectively always enabled.")
+        }
         val config = readConfig(
           cli.config.view,
           project = proj1,
@@ -330,8 +335,7 @@ object Main extends Logging {
         val config2 = DependencyResolver.modifyConfig(config.config, None, proj1.rootDir)
         ViashConfig.view(
           config2, 
-          format = cli.config.view.format(),
-          parseArgumentGroups = cli.config.view.parse_argument_groups()
+          format = cli.config.view.format()
         )
         0
       case List(cli.config, cli.config.inject) =>

diff --git a/src/main/scala/io/viash/ViashConfig.scala b/src/main/scala/io/viash/ViashConfig.scala
@@ -32,36 +32,13 @@ import io.viash.runners.Runner
 
 object ViashConfig extends Logging{
 
-  def view(config: Config, format: String, parseArgumentGroups: Boolean): Unit = {
-    val conf0 = 
-      if (parseArgumentGroups) {
-        config.copy(
-          functionality = config.functionality.copy(
-            arguments = Nil,
-            argument_groups = config.functionality.allArgumentGroups,
-          )
-        )
-      } else {
-        config
-      }
-    val json = ConfigMeta.configToCleanJson(conf0)
+  def view(config: Config, format: String): Unit = {
+    val json = ConfigMeta.configToCleanJson(config)
     infoOut(json.toFormattedString(format))
   }
 
-  def viewMany(configs: List[Config], format: String, parseArgumentGroups: Boolean): Unit = {
-    val confs0 = configs.map{ config => 
-      if (parseArgumentGroups) {
-        config.copy(
-          functionality = config.functionality.copy(
-            arguments = Nil,
-            argument_groups = config.functionality.allArgumentGroups,
-          )
-        )
-      } else {
-        config
-      }
-    }
-    val jsons = confs0.map(c => ConfigMeta.configToCleanJson(c))
+  def viewMany(configs: List[Config], format: String): Unit = {
+    val jsons = configs.map(c => ConfigMeta.configToCleanJson(c))
     infoOut(jsons.asJson.toFormattedString(format))
   }
 

diff --git a/src/main/scala/io/viash/ViashNamespace.scala b/src/main/scala/io/viash/ViashNamespace.scala
@@ -279,12 +279,11 @@ object ViashNamespace extends Logging {
 
   def list(
     configs: List[AppliedConfig], 
-    format: String = "yaml", 
-    parseArgumentGroups: Boolean
+    format: String = "yaml"
   ): Unit = {
     val configs2 = configs.filter(_.status.isEmpty).map(_.config)
 
-    ViashConfig.viewMany(configs2, format, parseArgumentGroups)
+    ViashConfig.viewMany(configs2, format)
 
     printResults(configs.map(ac => ac.status.getOrElse(Success)), false, false)
   }

diff --git a/src/main/scala/io/viash/ViashTest.scala b/src/main/scala/io/viash/ViashTest.scala
@@ -25,7 +25,7 @@ import java.time.temporal.ChronoUnit
 import scala.util.Random
 
 import config.Config
-import functionality.Functionality
+import functionality.{Functionality, ArgumentGroup}
 import functionality.arguments.{FileArgument, Output}
 import functionality.resources.{BashScript, Script}
 import helpers.{IO, Logging, LoggerOutput, LoggerLevel}
@@ -251,8 +251,7 @@ object ViashTest extends Logging {
             namespace = fun.namespace,
             version = fun.version,
             // set dirArg as argument so that Docker can chown it after execution
-            arguments = List(dirArg),
-            argument_groups = Nil,
+            argument_groups = List(ArgumentGroup("default", None, List(dirArg))),
             resources = List(test),
             set_wd_to_resources_dir = true
           ))(appliedConfig)

diff --git a/src/main/scala/io/viash/cli/CLIConf.scala b/src/main/scala/io/viash/cli/CLIConf.scala
@@ -284,7 +284,7 @@ class CLIConf(arguments: Seq[String]) extends ScallopConf(arguments) with Loggin
       val parse_argument_groups = registerOpt[Boolean](
         name = "parse_argument_groups",
         default = Some(false),
-        descr = "Whether or not to postprocess each component's @[argument groups](argument_groups)."
+        descr = "DEPRECATED. This is now always enabled. Whether or not to postprocess each component's @[argument groups](argument_groups)."
       )
     }
     val inject = new DocumentedSubcommand("inject") with ViashCommand with ViashLogger {
@@ -368,7 +368,7 @@ class CLIConf(arguments: Seq[String]) extends ScallopConf(arguments) with Loggin
       val parse_argument_groups = registerOpt[Boolean](
         name = "parse_argument_groups",
         default = Some(false),
-        descr = "Whether or not to postprocess each component's @[argument groups](argument_groups)."
+        descr = "DEPRECATED. This is now always enabled. Whether or not to postprocess each component's @[argument groups](argument_groups)."
       )
     }
 

diff --git a/src/main/scala/io/viash/functionality/Functionality.scala b/src/main/scala/io/viash/functionality/Functionality.scala
@@ -78,36 +78,6 @@ case class Functionality(
   @since("Viash 0.3.1")
   @default("Empty")
   authors: List[Author] = Nil,
-
-  @description(
-    """A list of @[arguments](argument) for this component. For each argument, a type and a name must be specified. Depending on the type of argument, different properties can be set. See these reference pages per type for more information:  
-      |
-      | - @[string](arg_string)
-      | - @[file](arg_file)
-      | - @[integer](arg_integer)
-      | - @[double](arg_double)
-      | - @[boolean](arg_boolean)
-      | - @[boolean_true](arg_boolean_true)
-      | - @[boolean_false](arg_boolean_false)
-      |""".stripMargin)
-  @example(
-    """arguments:
-      |  - name: --foo
-      |    type: file
-      |    alternatives: [-f]
-      |    description: Description of foo
-      |    default: "/foo/bar"
-      |    must_exist: true
-      |    direction: output
-      |    required: false
-      |    multiple: true
-      |    multiple_sep: ","
-      |  - name: --bar
-      |    type: string
-      |""".stripMargin,
-      "yaml")
-  @default("Empty")
-  arguments: List[Argument[_]] = Nil,
 
   @description(
     """A grouping of the @[arguments](argument), used to display the help message.
@@ -287,29 +257,39 @@ case class Functionality(
   @internalFunctionality
   set_wd_to_resources_dir: Boolean = false
 ) {
+  // Handled in preparsing
+  @description(
+    """A list of @[arguments](argument) for this component. For each argument, a type and a name must be specified. Depending on the type of argument, different properties can be set. See these reference pages per type for more information:  
+      |
+      | - @[string](arg_string)
+      | - @[file](arg_file)
+      | - @[integer](arg_integer)
+      | - @[double](arg_double)
+      | - @[boolean](arg_boolean)
+      | - @[boolean_true](arg_boolean_true)
+      | - @[boolean_false](arg_boolean_false)
+      |""".stripMargin)
+  @example(
+    """arguments:
+      |  - name: --foo
+      |    type: file
+      |    alternatives: [-f]
+      |    description: Description of foo
+      |    default: "/foo/bar"
+      |    must_exist: true
+      |    direction: output
+      |    required: false
+      |    multiple: true
+      |    multiple_sep: ","
+      |  - name: --bar
+      |    type: string
+      |""".stripMargin,
+      "yaml")
+  @default("Empty")
+  private val arguments: List[Argument[_]] = Nil
 
-  // Combine inputs, outputs and arguments into one combined list
-  def allArguments = arguments ::: argument_groups.flatMap(arg => arg.arguments)
-
-  def allArgumentGroups: List[ArgumentGroup] = {
-    if (arguments.isEmpty) {
-      // if there are no arguments, just return the argument groups as is
-      argument_groups
-    } else if (argument_groups.exists(_.name == "Arguments")) {
-      // if there is already an argument group named 'Arguments', extend it with the arguments
-      argument_groups.map{
-        case gr if gr.name == "Arguments" =>
-          gr.copy(arguments = gr.arguments ::: arguments)
-        case gr => gr
-      }
-    } else {
-      // else create a new argument group
-      argument_groups ::: List(ArgumentGroup(
-        name = "Arguments",
-        arguments = arguments
-      ))
-    }
-  }
+  // Combine all arguments into one combined list
+  def allArguments = argument_groups.flatMap(arg => arg.arguments)
 
   // check whether there are not multiple positional arguments with multiplicity >1
   // and if there is one, whether its position is last

diff --git a/src/main/scala/io/viash/functionality/package.scala b/src/main/scala/io/viash/functionality/package.scala
@@ -22,6 +22,7 @@ import io.circe.{Decoder, Encoder, Json}
 import io.circe.ACursor
 
 import io.viash.helpers.Logging
+import io.circe.JsonObject
 
 package object functionality extends Logging {
   // import implicits
@@ -39,8 +40,69 @@ package object functionality extends Logging {
   // encoder and decoder for Functionality
   implicit val encodeFunctionality: Encoder.AsObject[Functionality] = deriveConfiguredEncoderStrict[Functionality]
 
-  // add file & direction defaults for inputs & outputs
-  implicit val decodeFunctionality: Decoder[Functionality] = deriveConfiguredDecoderFullChecks
+  // merge arguments and argument_groups into argument_groups
+  implicit val decodeFunctionality: Decoder[Functionality] = deriveConfiguredDecoderFullChecks[Functionality]
+    .prepare {
+      _.withFocus{ json =>
+        json.asObject match {
+          case None => json
+          case Some(jo) => 
+            val arguments = jo.apply("arguments")
+            val argument_groups = jo.apply("argument_groups")
+
+            val newJsonObject = (arguments, argument_groups) match {
+              case (None, _) => jo
+              case (Some(args), None) => 
+                jo.add("argument_groups", Json.fromValues(List(
+                  Json.fromJsonObject(
+                    JsonObject(
+                      "name" -> Json.fromString("Arguments"),
+                      "arguments" -> args
+                    )
+                  )
+                )))
+              case (Some(args), Some(arg_groups)) =>
+                // determine if we should prepend or append arguments to argument_groups
+                val prepend = jo.keys.find(s => s == "argument_groups" || s == "arguments") == Some("arguments")
+                def combinerSeq(a: Vector[Json], b: Seq[Json]) = if (prepend) b ++: a else a :++ b
+                def combiner(a: Vector[Json], b: Json) = if (prepend) b +: a else a :+ b
+
+                // get the argument group named 'Arguments' from arg_groups
+                val argumentsGroup = arg_groups.asArray.flatMap(_.find(_.asObject.exists(_.apply("name").exists(_ == Json.fromString("Arguments")))))
+                argumentsGroup match {
+                  case None =>
+                    // no argument_group with name 'Argument' exists, so just add arguments as a new argument group
+                    jo.add("argument_groups", Json.fromValues(
+                      combiner(
+                        arg_groups.asArray.get,
+                        Json.fromJsonObject(
+                          JsonObject(
+                            "name" -> Json.fromString("Arguments"),
+                            "arguments" -> args
+                          )
+                        )
+                      )
+                    ))
+                  case Some(ag) =>
+                    // argument_group with name 'Argument' exists, so add arguments to this argument group
+                    val newAg = ag.asObject.get.add("arguments",
+                      Json.fromValues(
+                        combinerSeq(
+                          ag.asObject.get.apply("arguments").get.asArray.get,
+                          args.asArray.get
+                        )
+                      )
+                    )
+                    jo.add("argument_groups", Json.fromValues(arg_groups.asArray.get.map{
+                      case ag if ag == argumentsGroup.get => Json.fromJsonObject(newAg)
+                      case ag => ag
+                    }))
+                }
+            }
+            Json.fromJsonObject(newJsonObject.remove("arguments"))
+        }
+
+    }}
 
   // encoder and decoder for Author
   implicit val encodeAuthor: Encoder.AsObject[Author] = deriveConfiguredEncoder
@@ -52,37 +114,7 @@ package object functionality extends Logging {
 
   // encoder and decoder for ArgumentGroup
   implicit val encodeArgumentGroup: Encoder.AsObject[ArgumentGroup] = deriveConfiguredEncoder
-  implicit val decodeArgumentGroup: Decoder[ArgumentGroup] = deriveConfiguredDecoder[ArgumentGroup]
-    .validate(
-      validator[ArgumentGroup],
-      s"Could not convert json to ArgumentGroup."
-    )
-    .prepare {
-      checkDeprecation[ArgumentGroup](_) // check for deprecations
-      .withFocus(_.mapObject{ ag0 =>
-
-        // Check whether arguments contains a string value instead of an object. The support for this was removed in Viash 0.7.0
-        ag0.apply("arguments") match {
-          case Some(args) =>
-            args.mapArray(argVector => {
-              for (arg <- argVector) {
-                if (arg.isString) {
-                  info(
-                    s"""Error: specifying strings in the .argument field of argument group '${ag0.apply("name").get.asString.get}' was removed.
-                      |The .arguments field of an argument group should only contain arguments.
-                      |To solve this issue, copy the argument ${arg} directly into the argument group.""".stripMargin)
-                }
-              }
-              argVector
-            })
-          case _ => None
-        }
-
-        ag0
-      }
-      )
-    }
-
+  implicit val decodeArgumentGroup: Decoder[ArgumentGroup] = deriveConfiguredDecoderFullChecks
 
   // encoder and decoder for Status, make string lowercase before decoding
   implicit val encodeStatus: Encoder[Status] = Encoder.encodeEnumeration(Status)

diff --git a/src/main/scala/io/viash/helpers/Helper.scala b/src/main/scala/io/viash/helpers/Helper.scala
@@ -41,7 +41,7 @@ object Helper {
     val usageStr = functionality.usage.map("\n\nUsage:\n" + _.trim).getOrElse("")
 
     // PART 4: Options
-    val argGroupStrs = functionality.allArgumentGroups.map{argGroup =>
+    val argGroupStrs = functionality.argument_groups.map{argGroup =>
       val name = argGroup.name
       val descriptionStr = argGroup.description.map{
         des => "\n    " + Format.paragraphWrap(des.trim, maxWidth-4).mkString("\n    ") + "\n"

diff --git a/src/main/scala/io/viash/schemas/JsonSchema.scala b/src/main/scala/io/viash/schemas/JsonSchema.scala
@@ -130,7 +130,11 @@ object JsonSchema {
     val thisParameter = getThisParameter(info)
     val description = removeMarkup(thisParameter.description.get)
     val subclass = thisParameter.subclass.map(l => l.head)
-    val properties = info.filter(p => !p.name.startsWith("__")).filter(p => !p.removed.isDefined && (!config.strict || !p.deprecated.isDefined))
+    val properties = 
+      info
+      .filter(p => !p.name.startsWith("__")) // remove __this__
+      .filter(p => !p.removed.isDefined && (!config.strict || !p.deprecated.isDefined)) // always remove 'removed' arguments and if need be create a strict schema by removing deprecated
+      .filter(p => !config.strict || !(p.name == "arguments" && thisParameter.`type` == "Functionality")) // exception: remove 'arguments' in 'Functionality' for strict schema
     val propertiesJson = properties.map(p => {
       val pDescription = p.description.map(s => removeMarkup(s))
       val trimmedType = p.`type` match {