From 1e79c8c77e29f0aa5e6377d77ffa68d808ca4c89 Mon Sep 17 00:00:00 2001 From: Mark Einon Date: Wed, 3 Feb 2016 09:46:09 +0000 Subject: [PATCH 1/3] Optionally pass dataset lists as a single file within tool wrappers This change introduces an extra boolean XML attribute 'pass_as_file', that when used with an input parameter already having the 'multiple=True' attribute, passes the dataset list of files to the wrapped program as a single file containing a list of the dataset files. --- lib/galaxy/tools/evaluation.py | 28 ++++++++++++++++++++-------- lib/galaxy/tools/parameters/basic.py | 11 ++++++++++- lib/galaxy/tools/wrappers.py | 22 ++++++++++++++++++++++ 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 6e845b94b70..e6c0121cde7 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -11,6 +11,7 @@ from galaxy.tools.wrappers import ( ToolParameterValueWrapper, DatasetFilenameWrapper, DatasetListWrapper, + DatasetListAsFileWrapper, DatasetCollectionWrapper, SelectToolParameterWrapper, InputValueWrapper, @@ -130,7 +131,7 @@ class ToolEvaluator( object ): param_dict.update( incoming ) input_dataset_paths = dataset_path_rewrites( input_paths ) - self.__populate_wrappers(param_dict, input_dataset_paths) + self.__populate_wrappers(param_dict, input_dataset_paths, job_working_directory) self.__populate_input_dataset_wrappers(param_dict, input_datasets, input_dataset_paths) self.__populate_output_dataset_wrappers(param_dict, output_datasets, output_paths, job_working_directory) self.__populate_output_collection_wrappers(param_dict, output_collections, output_paths, job_working_directory) @@ -165,18 +166,29 @@ class ToolEvaluator( object ): do_walk( inputs, input_values ) - def __populate_wrappers(self, param_dict, input_dataset_paths): + def __populate_wrappers(self, param_dict, input_dataset_paths, job_working_directory): def wrap_input( input_values, input ): if isinstance( input, DataToolParameter ) and input.multiple: value = input_values[ input.name ] dataset_instances = DatasetListWrapper.to_dataset_instances( value ) - input_values[ input.name ] = \ - DatasetListWrapper( dataset_instances, - dataset_paths=input_dataset_paths, - datatypes_registry=self.app.datatypes_registry, - tool=self.tool, - name=input.name ) + if input.pass_as_file: + input_values[ input.name ] = \ + DatasetListAsFileWrapper( job_working_directory, + dataset_instances, + dataset_paths=input_dataset_paths, + datatypes_registry=self.app.datatypes_registry, + tool=self.tool, + name=input.name ) + else: + input_values[ input.name ] = \ + DatasetListWrapper( dataset_instances, + dataset_paths=input_dataset_paths, + datatypes_registry=self.app.datatypes_registry, + tool=self.tool, + name=input.name ) + + elif isinstance( input, DataToolParameter ): # FIXME: We're populating param_dict with conversions when # wrapping values, this should happen as a separate diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 9d8822a4081..a2701499e74 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -830,6 +830,7 @@ class SelectToolParameter( ToolParameter ): input_source = ensure_input_source( input_source ) ToolParameter.__init__( self, tool, input_source ) self.multiple = input_source.get_bool( 'multiple', False ) + self.pass_as_file = input_source.get_bool( 'pass_as_file', False ) # Multiple selects are optional by default, single selection is the inverse. self.optional = input_source.parse_optional( self.multiple ) self.display = input_source.get( 'display', None ) @@ -1042,6 +1043,7 @@ class SelectToolParameter( ToolParameter ): d['display'] = self.display d['multiple'] = self.multiple + d['pass_as_file'] = self.pass_as_file return d @@ -1114,7 +1116,8 @@ class GenomeBuildParameter( SelectToolParameter ): 'options' : options, 'value' : value, 'display' : self.display, - 'multiple' : self.multiple + 'multiple' : self.multiple, + 'pass_as_file' : self.pass_as_file }) return d @@ -1743,6 +1746,7 @@ class DataToolParameter( BaseDataToolParameter ): self.validators.append( validation.MetadataValidator() ) self._parse_formats( trans, tool, input_source ) self.multiple = input_source.get_bool('multiple', False) + self.pass_as_file = input_source.get_bool('pass_as_file', False) self.min = input_source.get( 'min' ) self.max = input_source.get( 'max' ) if self.min: @@ -2153,6 +2157,7 @@ class DataToolParameter( BaseDataToolParameter ): d['extensions'] = extensions d['edam_formats'] = edam_formats d['multiple'] = self.multiple + d['pass_as_file'] = self.pass_as_file if self.multiple: # For consistency, should these just always be in the dict? d['min'] = self.min @@ -2209,6 +2214,7 @@ class DataCollectionToolParameter( BaseDataToolParameter ): collection_types = [t.strip() for t in collection_types.split(",")] self._collection_types = collection_types self.multiple = False # Accessed on DataToolParameter a lot, may want in future + self.pass_as_file = False self.is_dynamic = True self._parse_options( input_source ) # TODO: Review and test. @@ -2378,6 +2384,7 @@ class DataCollectionToolParameter( BaseDataToolParameter ): d = super( DataCollectionToolParameter, self ).to_dict( trans ) d['extensions'] = self.extensions d['multiple'] = self.multiple + d['pass_as_file'] = self.pass_as_file d['options'] = {'hda': [], 'hdca': []} # return default content if context is not available @@ -2442,6 +2449,7 @@ class LibraryDatasetToolParameter( ToolParameter ): input_source = ensure_input_source( input_source ) ToolParameter.__init__( self, tool, input_source ) self.multiple = input_source.get_bool( 'multiple', True ) + self.pass_as_file = input_source.get_bool( 'pass_as_file', False ) def get_html_field( self, trans=None, value=None, other_values={} ): return form_builder.LibraryField( self.name, value=value, trans=trans ) @@ -2525,6 +2533,7 @@ class LibraryDatasetToolParameter( ToolParameter ): def to_dict( self, trans, view='collection', value_mapper=None, other_values=None ): d = super( LibraryDatasetToolParameter, self ).to_dict( trans ) d['multiple'] = self.multiple + d['pass_as_file'] = self.pass_as_file return d parameter_types = dict( diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index f61cb1aca18..07c4cafb026 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -1,3 +1,4 @@ +import os import pipes from galaxy import exceptions from galaxy.util.none_like import NoneDataset @@ -298,6 +299,27 @@ class DatasetListWrapper( list, ToolParameterValueWrapper, HasDatasets ): return ','.join( map( str, self ) ) +class DatasetListAsFileWrapper( DatasetListWrapper ): + """ + A DatasetListAsFileWrapper is a DatasetListWrapper whose __str__() method + returns a file containing a list of the DatasetList files, creating it if + necessary. This is so a Dataset List can be passed in a command line + without overflowing the command line argument limit. + """ + def __init__( self, job_working_directory, datasets, dataset_paths=[], **kwargs ): + self.job_working_directory = job_working_directory + super(DatasetListAsFileWrapper, self).__init__(datasets, dataset_paths, **kwargs ) + + def __str__(self): + # TODO - use dataset ID for this filename + filename = "dataset_xx_filelist" + filepath = os.path.join( self.job_working_directory, filename) + if not os.path.isfile(filepath): + listfile = open( filepath, 'w' ) + listfile.write( super(DatasetListAsFileWrapper, self).__str__().replace(',', '\n') ) + listfile.close() + return filepath + class DatasetCollectionWrapper( ToolParameterValueWrapper, HasDatasets ): def __init__( self, has_collection, dataset_paths=[], **kwargs ): From 27618a319d9c3bac01484bad80494f4423c04673 Mon Sep 17 00:00:00 2001 From: Mark Einon Date: Wed, 3 Feb 2016 10:10:32 +0000 Subject: [PATCH 2/3] [Trivial] Whitespace fixes to please pylint --- lib/galaxy/tools/evaluation.py | 1 - lib/galaxy/tools/wrappers.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index e6c0121cde7..b1425de9ef3 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -188,7 +188,6 @@ class ToolEvaluator( object ): tool=self.tool, name=input.name ) - elif isinstance( input, DataToolParameter ): # FIXME: We're populating param_dict with conversions when # wrapping values, this should happen as a separate diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index 07c4cafb026..9ac7adec779 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -320,6 +320,7 @@ class DatasetListAsFileWrapper( DatasetListWrapper ): listfile.close() return filepath + class DatasetCollectionWrapper( ToolParameterValueWrapper, HasDatasets ): def __init__( self, has_collection, dataset_paths=[], **kwargs ): From 66350fb87100a8986a95c28f9b532564c98f0034 Mon Sep 17 00:00:00 2001 From: Mark Einon Date: Thu, 4 Feb 2016 20:21:41 +0000 Subject: [PATCH 3/3] Modify pass_as_file to apply to all Multiple parameters and add test case Change the pass_as_file XML attribute to a cheetah method call, and apply to all paramter types that can have Multiple=True set - those given in DatasetListWrappers and DatasetCollectionWrappers. Test case and modification suggestion provided by John Chilton --- lib/galaxy/tools/evaluation.py | 25 +++++--------- lib/galaxy/tools/parameters/basic.py | 9 ----- lib/galaxy/tools/parameters/wrapped.py | 4 ++- lib/galaxy/tools/wrappers.py | 38 +++++++-------------- test/functional/tools/paths_as_file.xml | 37 ++++++++++++++++++++ test/functional/tools/samples_tool_conf.xml | 1 + 6 files changed, 63 insertions(+), 51 deletions(-) create mode 100644 test/functional/tools/paths_as_file.xml diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index b1425de9ef3..4fa2eee3a12 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -11,7 +11,6 @@ from galaxy.tools.wrappers import ( ToolParameterValueWrapper, DatasetFilenameWrapper, DatasetListWrapper, - DatasetListAsFileWrapper, DatasetCollectionWrapper, SelectToolParameterWrapper, InputValueWrapper, @@ -172,21 +171,13 @@ class ToolEvaluator( object ): if isinstance( input, DataToolParameter ) and input.multiple: value = input_values[ input.name ] dataset_instances = DatasetListWrapper.to_dataset_instances( value ) - if input.pass_as_file: - input_values[ input.name ] = \ - DatasetListAsFileWrapper( job_working_directory, - dataset_instances, - dataset_paths=input_dataset_paths, - datatypes_registry=self.app.datatypes_registry, - tool=self.tool, - name=input.name ) - else: - input_values[ input.name ] = \ - DatasetListWrapper( dataset_instances, - dataset_paths=input_dataset_paths, - datatypes_registry=self.app.datatypes_registry, - tool=self.tool, - name=input.name ) + input_values[ input.name ] = \ + DatasetListWrapper( job_working_directory, + dataset_instances, + dataset_paths=input_dataset_paths, + datatypes_registry=self.app.datatypes_registry, + tool=self.tool, + name=input.name ) elif isinstance( input, DataToolParameter ): # FIXME: We're populating param_dict with conversions when @@ -245,6 +236,7 @@ class ToolEvaluator( object ): name=input.name ) wrapper = DatasetCollectionWrapper( + job_working_directory, dataset_collection, **wrapper_kwds ) @@ -317,6 +309,7 @@ class ToolEvaluator( object ): name=name ) wrapper = DatasetCollectionWrapper( + job_working_directory, out_collection, **wrapper_kwds ) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index a2701499e74..55889d268a9 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -830,7 +830,6 @@ class SelectToolParameter( ToolParameter ): input_source = ensure_input_source( input_source ) ToolParameter.__init__( self, tool, input_source ) self.multiple = input_source.get_bool( 'multiple', False ) - self.pass_as_file = input_source.get_bool( 'pass_as_file', False ) # Multiple selects are optional by default, single selection is the inverse. self.optional = input_source.parse_optional( self.multiple ) self.display = input_source.get( 'display', None ) @@ -1043,7 +1042,6 @@ class SelectToolParameter( ToolParameter ): d['display'] = self.display d['multiple'] = self.multiple - d['pass_as_file'] = self.pass_as_file return d @@ -1117,7 +1115,6 @@ class GenomeBuildParameter( SelectToolParameter ): 'value' : value, 'display' : self.display, 'multiple' : self.multiple, - 'pass_as_file' : self.pass_as_file }) return d @@ -1746,7 +1743,6 @@ class DataToolParameter( BaseDataToolParameter ): self.validators.append( validation.MetadataValidator() ) self._parse_formats( trans, tool, input_source ) self.multiple = input_source.get_bool('multiple', False) - self.pass_as_file = input_source.get_bool('pass_as_file', False) self.min = input_source.get( 'min' ) self.max = input_source.get( 'max' ) if self.min: @@ -2157,7 +2153,6 @@ class DataToolParameter( BaseDataToolParameter ): d['extensions'] = extensions d['edam_formats'] = edam_formats d['multiple'] = self.multiple - d['pass_as_file'] = self.pass_as_file if self.multiple: # For consistency, should these just always be in the dict? d['min'] = self.min @@ -2214,7 +2209,6 @@ class DataCollectionToolParameter( BaseDataToolParameter ): collection_types = [t.strip() for t in collection_types.split(",")] self._collection_types = collection_types self.multiple = False # Accessed on DataToolParameter a lot, may want in future - self.pass_as_file = False self.is_dynamic = True self._parse_options( input_source ) # TODO: Review and test. @@ -2384,7 +2378,6 @@ class DataCollectionToolParameter( BaseDataToolParameter ): d = super( DataCollectionToolParameter, self ).to_dict( trans ) d['extensions'] = self.extensions d['multiple'] = self.multiple - d['pass_as_file'] = self.pass_as_file d['options'] = {'hda': [], 'hdca': []} # return default content if context is not available @@ -2449,7 +2442,6 @@ class LibraryDatasetToolParameter( ToolParameter ): input_source = ensure_input_source( input_source ) ToolParameter.__init__( self, tool, input_source ) self.multiple = input_source.get_bool( 'multiple', True ) - self.pass_as_file = input_source.get_bool( 'pass_as_file', False ) def get_html_field( self, trans=None, value=None, other_values={} ): return form_builder.LibraryField( self.name, value=value, trans=trans ) @@ -2533,7 +2525,6 @@ class LibraryDatasetToolParameter( ToolParameter ): def to_dict( self, trans, view='collection', value_mapper=None, other_values=None ): d = super( LibraryDatasetToolParameter, self ).to_dict( trans ) d['multiple'] = self.multiple - d['pass_as_file'] = self.pass_as_file return d parameter_types = dict( diff --git a/lib/galaxy/tools/parameters/wrapped.py b/lib/galaxy/tools/parameters/wrapped.py index becf89f4872..c97692f46ea 100644 --- a/lib/galaxy/tools/parameters/wrapped.py +++ b/lib/galaxy/tools/parameters/wrapped.py @@ -58,7 +58,8 @@ class WrappedParameters( object ): value = input_values[ input.name ] dataset_instances = DatasetListWrapper.to_dataset_instances( value ) input_values[ input.name ] = \ - DatasetListWrapper( dataset_instances, + DatasetListWrapper( None, + dataset_instances, datatypes_registry=trans.app.datatypes_registry, tool=tool, name=input.name ) @@ -72,6 +73,7 @@ class WrappedParameters( object ): input_values[ input.name ] = SelectToolParameterWrapper( input, input_values[ input.name ], tool.app, other_values=incoming ) elif isinstance( input, DataCollectionToolParameter ): input_values[ input.name ] = DatasetCollectionWrapper( + None, input_values[ input.name ], datatypes_registry=trans.app.datatypes_registry, tool=tool, diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index 9ac7adec779..f0a7ca50f87 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -1,5 +1,6 @@ import os import pipes +import tempfile from galaxy import exceptions from galaxy.util.none_like import NoneDataset from galaxy.util import odict @@ -264,11 +265,18 @@ class HasDatasets: wrapper_kwds[ "dataset_path" ] = dataset_paths[ real_path ] return DatasetFilenameWrapper( dataset, **wrapper_kwds ) + def paths_as_file(self, sep="\n"): + handle, filepath = tempfile.mkstemp(prefix="gx_file_list", dir=self.job_working_directory) + contents = sep.join(map(str, self)) + os.write(handle, contents) + os.close(handle) + return filepath + class DatasetListWrapper( list, ToolParameterValueWrapper, HasDatasets ): """ """ - def __init__( self, datasets, dataset_paths=[], **kwargs ): + def __init__( self, job_working_directory, datasets, dataset_paths=[], **kwargs ): if not isinstance(datasets, list): datasets = [datasets] @@ -280,6 +288,7 @@ class DatasetListWrapper( list, ToolParameterValueWrapper, HasDatasets ): return self._dataset_wrapper( dataset, dataset_paths, **kwargs ) list.__init__( self, map( to_wrapper, datasets ) ) + self.job_working_directory = job_working_directory @staticmethod def to_dataset_instances( dataset_instance_sources ): @@ -299,32 +308,11 @@ class DatasetListWrapper( list, ToolParameterValueWrapper, HasDatasets ): return ','.join( map( str, self ) ) -class DatasetListAsFileWrapper( DatasetListWrapper ): - """ - A DatasetListAsFileWrapper is a DatasetListWrapper whose __str__() method - returns a file containing a list of the DatasetList files, creating it if - necessary. This is so a Dataset List can be passed in a command line - without overflowing the command line argument limit. - """ - def __init__( self, job_working_directory, datasets, dataset_paths=[], **kwargs ): - self.job_working_directory = job_working_directory - super(DatasetListAsFileWrapper, self).__init__(datasets, dataset_paths, **kwargs ) - - def __str__(self): - # TODO - use dataset ID for this filename - filename = "dataset_xx_filelist" - filepath = os.path.join( self.job_working_directory, filename) - if not os.path.isfile(filepath): - listfile = open( filepath, 'w' ) - listfile.write( super(DatasetListAsFileWrapper, self).__str__().replace(',', '\n') ) - listfile.close() - return filepath - - class DatasetCollectionWrapper( ToolParameterValueWrapper, HasDatasets ): - def __init__( self, has_collection, dataset_paths=[], **kwargs ): + def __init__( self, job_working_directory, has_collection, dataset_paths=[], **kwargs ): super(DatasetCollectionWrapper, self).__init__() + self.job_working_directory = job_working_directory if has_collection is None: self.__input_supplied = False @@ -353,7 +341,7 @@ class DatasetCollectionWrapper( ToolParameterValueWrapper, HasDatasets ): element_identifier = dataset_collection_element.element_identifier if dataset_collection_element.is_collection: - element_wrapper = DatasetCollectionWrapper( dataset_collection_element, dataset_paths, **kwargs ) + element_wrapper = DatasetCollectionWrapper(job_working_directory, dataset_collection_element, dataset_paths, **kwargs ) else: element_wrapper = self._dataset_wrapper( element_object, dataset_paths, **kwargs) diff --git a/test/functional/tools/paths_as_file.xml b/test/functional/tools/paths_as_file.xml new file mode 100644 index 00000000000..b6dd3b00647 --- /dev/null +++ b/test/functional/tools/paths_as_file.xml @@ -0,0 +1,37 @@ + + + + + $out1 + ]]> + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index 553c704f15c..48084ab8ff6 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -44,6 +44,7 @@ +