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

Fix element_identifier unavailable with data_collection input param #2570

Merged
merged 5 commits into from Jul 7, 2016

Conversation

Projects
None yet
4 participants
@abretaud
Copy link
Contributor

commented Jun 28, 2016

Here's a (failing) test case for galaxyproject/tools-iuc/issues/857
I also added a small fix for an UnboundLocalError I encountered while writing it

@abretaud abretaud changed the title WIP: element_identifier unavailable with conditional Element_identifier unavailable with data_collection input param Jun 29, 2016

@abretaud abretaud changed the title Element_identifier unavailable with data_collection input param Fix element_identifier unavailable with data_collection input param Jun 29, 2016

@abretaud

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2016

It should be fixed now, and it turns out it had nothing to do with conditionals

@@ -307,7 +307,7 @@ def to_wrapper( dataset ):
element = dataset
dataset = element.dataset_instance
kwargs["identifier"] = element.element_identifier
return self._dataset_wrapper( dataset, dataset_paths, **kwargs )
return self._dataset_wrapper( dataset, dataset_paths, identifier = element_identifier, **kwargs )

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Jun 29, 2016

Member

I don't think is needed, should be already done at line 309.

create = create_response.json()
outputs = create[ 'outputs' ]
jobs = create[ 'jobs' ]
implicit_collections = create[ 'implicit_collections' ]

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Jun 29, 2016

Member

This variable is never used, can be removed.

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Jul 7, 2016

This looks good to me, should we merge @nsoranzo ?

@@ -0,0 +1,15 @@
<tool id="identifier_conditional" name="identifier_conditional">

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Jul 7, 2016

Member

Should this id be identifier_collection? Seems more proper and it's what is used at https://github.com/galaxyproject/galaxy/pull/2570/files#diff-46fef38dde830e533e91e849d89bceabR809

If yes, also this file should be renamed and the change propagated to test/functional/tools/samples_tool_conf.xml

#end for
</command>
<inputs>
<param type="data_collection" collection_type="list" name="input1" label="Input 1" multiple="true" />

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Jul 7, 2016

Member

Remove multiple="true" ? Or is this supporting multiple dataset lists?

@nsoranzo nsoranzo merged commit 05eebac into galaxyproject:dev Jul 7, 2016

4 checks passed

api test Build finished. 220 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 110 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 582 tests run, 0 skipped, 0 failed.
Details
@nsoranzo

This comment has been minimized.

Copy link
Member

commented Jul 7, 2016

Thanks @abretaud!

@abretaud

This comment has been minimized.

Copy link
Contributor Author

commented Jul 7, 2016

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.