Some small things to improve

[bgruening]

This is a small list of things that can make the wrappers a bit better. All of those are not important for the first version I think.

    <param name="seqs" type="data" format="qza" label="seqs: MultiplexedPairedEndBarcodeInSequence" help="[required]  The paired-end sequences to be demultiplexed.">

The [required] string can go away. If you do not use optional="true" everything is required and Galaxy will make sure the user knows about it.

    <section name="__q2galaxy__GUI__section__extra_opts__" title="Click here for additional options">

The __ might be confusing down the line. E.g. this will appear in Galaxy workflows etc and might be confusing for users that deal with it.

        <repeat name="front_f" help="[optional]  Sequence of an adapter ligated to the 5' end. The adapter and any preceding bases are trimmed. Partial matches at the 5' end are allowed. If a `^` character is prepended, the adapter is only found if it is at the beginning of the read. Search in forward read." title="front_f: List[Str]">

The title does not look nice. [optional] should be removed

            <param name="column" type="text" label="Column Name">

Should this not be a column parameter?

        <param name="minimum_length" type="integer" min="1" value="1" label="minimum_length: Int % Range(1, None)" help="[default: 1]  Discard reads shorter than specified value. Note, the cutadapt default of 0 has been overridden, because that value produces empty sequence records."/>

The [default: 1] can be removed, it is already encoded in the UI.

[ebolyen]

Thanks for the close review @bgruening! (And sorry for the delayed response, I was out last week.)

---

The [required] string can go away. If you do not use optional="true" everything is required and Galaxy will make sure the user knows about it.

[optional] should be removed

The [default: 1] can be removed, it is already encoded in the UI.

These were kind of workarounds for the Galaxy UI from a while ago: if you changed or deleted the argument, it wasn't clear what the default would have been, so adding it to the help text in a uniform way seemed best at the time. I'll take another look and figure out how better to deal with that. If there isn't some way of handling it, what would be the best way to get started with trying to add it to the Galaxy UI?

---

The title does not look nice.

I wasn't sure where the best place to put our types would be. So I opted for something that made visual sense for the form's hierarchy. This is a point where you definitely feel the framework-inside-a-framework problem. I could put this in the help instead, but I think it's equally as important as the input's label.

---

The __ might be confusing down the line. E.g. this will appear in Galaxy workflows etc and might be confusing for users that deal with it.

Yeah 100%, I was bummed when I realized these actually showed up in the workflows. Haven't come up with a great solution here as these aren't "real" parameters so to speak, and I was using the predictable structure to quickly flatten the resulting nested arguments. Open to suggestions!

Relevant code: https://github.com/qiime2/q2galaxy/blob/master/q2galaxy/core/util.py#L157-L168 https://github.com/qiime2/q2galaxy/blob/master/q2galaxy/__main__.py#L111-L117

---

Should this not be a column parameter?

Generally speaking, it is, but this particular action must be giving us a type of Str instead of MetadataColumn[Categorical | Numeric]. When a plugin annotates the type correctly, we'll produce the following:

<conditional name="metadata">
  <param name="type" type="select" label="metadata: MetadataColumn[Categorical]" help="[required]  Categorical sample metadata column.">
      <option value="tsv" selected="true">Metadata from TSV</option>
      <option value="qza">Metadata from Artifact</option>
    </param>
    <when value="tsv">
        <param name="source" type="data" format="tabular,qiime2.tabular" label="Metadata Source"/>
        <param name="column" type="data_column" label="Column Name" data_ref="source" use_header_names="true">
            <validator type="expression" message="The first column cannot be selected (they are IDs).">value != "1"</validator>
        </param>
    </when>
    <when value="qza">
        <param name="source" type="data" format="qza" label="Metadata Source"/>
        <param name="column" type="text" label="Column Name">
            <validator type="empty_field"/>
        </param>
    </when>
</conditional>

Note that there is a column with type of text, but that is only when the source is a QZA file. These may not strictly be tabular in nature, but have some transformation (defined by a QIIME 2 plugin) which can make it tabular. As we don't know the column names ahead of time, this looks a bit awkward. I had imagined potentially "pre-calculating" the columns and then trying to set the needed Galaxy Metadata attributes on the QIIME2Artifact datatype such that tool output actions could pick up some supplementary file and set the appropriate columns. But I must admit I got a bit lost trying to figure out how the data_column actually picked up the column names, so abandoned it for now.

Hopefully having Galaxy as an interface will motivate some improvements on our end here (we probably need to make it more convenient to handle multiple columns from the same source on the command line before everyone gets board).

[ebolyen]

@bgruening an additional problem is tool-ids, we generate bad ones it seems.

1st approach: qiime2.feature_table.filter_samples But I found a thread somewhere saying periods should not be permitted, so I did the 2nd approach.

2nd approach: qiime2_feature-table_filter-samples Everything worked until I tried to use the toolshed, and so I want to use this 3rd approach:

qiime2__feature_table__filter_samples It's not super pretty, but it let's me back out the relevant pieces from the ID which came up often enough as a problem that I may need some kind of pattern here. Am I going to regret going this route?