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

Sort of bug or at least unexpected behaviour #55

Open
lldelisle opened this issue Oct 21, 2022 · 10 comments
Open

Sort of bug or at least unexpected behaviour #55

lldelisle opened this issue Oct 21, 2022 · 10 comments

Comments

@lldelisle
Copy link

Hi,
I downloaded the ga from usegalaxy.org and I got:
image
I guess "label": null means that it is not a workflow output right.

@davelopez
Copy link
Owner

The IWC profile requires defining a label for each workflow output. Since the label value is null is considered as not really defined.

The workflow is valid itself, if you go back to the basic validation profile it will not show this as an error. This only means that galaxy doesn't enforce IWC best practices workflows right now :)

@lldelisle
Copy link
Author

But this is not a workflow output... This is a workflow input parameter...

@lldelisle
Copy link
Author

And it seems that sometimes it is defined and sometimes not... I don't know why... Maybe if the type is data_input or parameter_input or data_collection_input the workflow_output should not be checked...

@lldelisle
Copy link
Author

Or this needs to be directly in galaxy code... I don't know.

@davelopez
Copy link
Owner

But this is not a workflow output... This is a workflow input parameter...

I'm a bit confused and out of context here, also I'm no expert in workflows myself so please excuse me 😅

In your screenshot, at line 66 looks like this output is defined in the workflow_outputs section, that's what makes the extension consider this a workflow output without a label.

Is your concern that Galaxy is not exporting your workflow as you expect? or that the rule of "all workflow outputs must define a label" is not right?

@lldelisle
Copy link
Author

Sorry, my explainations are unclear.

  • I will try to better explain what is happening:
    When you create a input box in the workflow editor (data_input or parameter_input or data_collection_input), then sometimes when you download the ga file you have:
            "workflow_outputs": [
                {
                    "label": null,
                    "output_name": "output",
                    "uuid": "d4a5d221-29f4-469d-ad39-63c56bab4e8c"
                }
            ]

And sometimes you have:

            "workflow_outputs": []

(See galaxyproject/iwc@d7fa5bc).
Both works and give the same results.

  • What is my concern:
    • Galaxy should never write non empty workflow_outputs for inputs.
    • Because it does, maybe the linter could adapt and do not consider as error the fact that an input has a non labelled workflow_outputs.

Is it clearer?

@davelopez
Copy link
Owner

davelopez commented Oct 21, 2022

Thank you so much for the clarification!
So If now I understand it correctly:

Galaxy should never write non empty workflow_outputs for inputs.

Yep, this sounds like a bug in Galaxy, might be worth reporting it there if it is not already.

Because it does, maybe the linter could adapt and do not consider as error the fact that an input has a non labelled workflow_outputs.

I'm not so sure about this part... I think it is better to fix the underlying issue in Galaxy than kind of hide it on the linter side. What do you think @mvdbeek or @wm75?

@wm75
Copy link
Collaborator

wm75 commented Oct 21, 2022

AFAIK, a label: null in the workflow_outputs of an input param does not get reported as a Best Practice violation.
@mvdbeek has repeatedly argued that inputs are outputs though so having the workflow_outputs section present seems justified.
otoh, then the labels within should probably not be null by default?

@wm75
Copy link
Collaborator

wm75 commented Oct 21, 2022

So @davelopez, imho, it's a feature not a bug if the extension complains about the null value here even if that goes beyond what the workflow editor does currently.

@lldelisle
Copy link
Author

lldelisle commented Oct 21, 2022

@wm75 to be sure I understand, you would say:

  • having a workflow_outputs section in input is an expected behaviour from galaxy. However the choice of label: null is maybe not ideal.
    • Is the fact that this section disappear when you upload a workflow a bug from galaxy?
    • Should I close the issue I just opened?
  • The galaxy workflow iwc linter should not mark workflow_outputs with label: null as errors.

Is it correct?

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