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.

Sign up for GitHub

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

Jump to bottom

Ensure tags are copied and presented when moving Collections #4277

Merged
merged 6 commits into from Jul 14, 2017
Merged

Ensure tags are copied and presented when moving Collections #4277

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
37ef9a9
Copy tags on HDCA, add tags when dictifying
pvanheus Jul 5, 2017
f0e5ec1
Merge branch 'dev' of github.com:pvanheus/galaxy into dev
pvanheus Jul 5, 2017
37cb522
copy tags using tag manager
pvanheus Jul 5, 2017
bf8b156
Move tag copying into HasTags mixin
pvanheus Jul 5, 2017
484d581
Remove inheritance from object in HasTags
pvanheus Jul 6, 2017
35d67ea
Put back inheritance from object in HasTags
pvanheus Jul 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/galaxy/managers/collections.py
View file
Open in desktop
Expand Up @@ -188,6 +188,8 @@ def copy( self, trans, parent, source, encoded_source_id ):
assert source == "hdca" # for now
source_hdca = self.__get_history_collection_instance( trans, encoded_source_id )
new_hdca = source_hdca.copy()
tags_str = self.tag_manager.get_tags_str(source_hdca.tags)
self.tag_manager.apply_item_tags(trans.get_user(), new_hdca, tags_str)
parent.add_dataset_collection( new_hdca )
trans.sa_session.add( new_hdca )
trans.sa_session.flush()
Expand Down
64 changes: 35 additions & 29 deletions lib/galaxy/model/__init__.py
View file
Open in desktop
Expand Up @@ -83,6 +83,26 @@ def set_datatypes_registry( d_registry ):
_datatypes_registry = d_registry


class HasTags( object ):
dict_collection_visible_keys = ( 'tags' )
dict_element_visible_keys = ( 'tags' )

def to_dict(self, *args, **kwargs):
rval = super( HasTags, self).to_dict(*args, **kwargs)
rval['tags'] = self.make_tag_string_list()
return rval

def make_tag_string_list(self):
# add tags string list
tags_str_list = []
for tag in self.tags:
tag_str = tag.user_tname
if tag.value is not None:
tag_str += ":" + tag.user_value
tags_str_list.append( tag_str )
return tags_str_list


class HasName:

def get_display_name( self ):
Expand Down Expand Up @@ -1157,7 +1177,7 @@ def is_hda(d):
return isinstance( d, HistoryDatasetAssociation )


class History( object, Dictifiable, UsesAnnotations, HasName ):
class History( HasTags, Dictifiable, UsesAnnotations, HasName ):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works the same way, but I prefer the more explicit object, if we could leave that here. HasTags must come before Dictifiable right? MRO mess is why we may have pulled back from some of these mixins, IIRC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason it was done this was is because HasTags inherits from object and calls super() in order to let Dictifiable create the original dictionary (to which it adds the 'tags' key). I changed it in my latest commit to not inherit from object and use super() so will see if that breaks anything. Specifically I'm concerned about the

This stuff certainly isn't straightforward to navigate in Python. If I understand it correctly, there should be a path from the subclass to_dict() up through the inheritance chain so that the last thing mentioned in the list is the one that actually creates the dictionary, everything else before it uses super() to get a dictionary and then adds to that dict.

My latest change has broken that principle so it might have broken the chain, since now HasTags and Dictifiable both have a to_dict() method that creates a dictionary. Not sure if the testing will catch this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted back to the HasTags inherits from object way of doing things and then removed object from the inheritance list. Looks like the only way to tame the MRO beast.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrmm. Yeah, my concern is that that approach isn't scalable for mixins. I'll tinker with a few things and see what I can come up with.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. In the meantime 37cb522 is the simple solution for the bug this addresses.


dict_collection_visible_keys = ( 'id', 'name', 'published', 'deleted' )
dict_element_visible_keys = ( 'id', 'name', 'genome_build', 'deleted', 'purged', 'update_time',
Expand Down Expand Up @@ -1342,15 +1362,6 @@ def to_dict( self, view='collection', value_mapper=None ):
# Get basic value.
rval = super( History, self ).to_dict( view=view, value_mapper=value_mapper )

# Add tags.
tags_str_list = []
for tag in self.tags:
tag_str = tag.user_tname
if tag.value is not None:
tag_str += ":" + tag.user_value
tags_str_list.append( tag_str )
rval[ 'tags' ] = tags_str_list

if view == 'element':
rval[ 'size' ] = int( self.disk_size )

Expand Down Expand Up @@ -2327,7 +2338,8 @@ def convert_dataset( self, trans, target_type ):
return msg


class HistoryDatasetAssociation( DatasetInstance, Dictifiable, UsesAnnotations, HasName ):
class HistoryDatasetAssociation( DatasetInstance, HasTags, Dictifiable, UsesAnnotations,
HasName ):
"""
Resource class that creates a relation between a dataset and a user history.
"""
Expand Down Expand Up @@ -2501,6 +2513,7 @@ def to_dict( self, view='collection', expose_dataset_path=False ):
# Since this class is a proxy to rather complex attributes we want to
# display in other objects, we can't use the simpler method used by
# other model classes.
original_rval = super( HistoryDatasetAssociation, self ).to_dict(view=view)
hda = self
rval = dict( id=hda.id,
hda_ldda='hda',
Expand All @@ -2523,14 +2536,7 @@ def to_dict( self, view='collection', expose_dataset_path=False ):
misc_info=hda.info.strip() if isinstance( hda.info, string_types ) else hda.info,
misc_blurb=hda.blurb )

# add tags string list
tags_str_list = []
for tag in self.tags:
tag_str = tag.user_tname
if tag.value is not None:
tag_str += ":" + tag.user_value
tags_str_list.append( tag_str )
rval[ 'tags' ] = tags_str_list
rval.update(original_rval)

if hda.copied_from_library_dataset_dataset_association is not None:
rval['copied_from_ldda_id'] = hda.copied_from_library_dataset_dataset_association.id
Expand Down Expand Up @@ -3318,7 +3324,10 @@ def set_from_dict( self, new_data ):
return changed


class HistoryDatasetCollectionAssociation( DatasetCollectionInstance, UsesAnnotations, Dictifiable ):
class HistoryDatasetCollectionAssociation( DatasetCollectionInstance,
HasTags,
Dictifiable,
UsesAnnotations ):
""" Associates a DatasetCollection with a History. """
editable_keys = ( 'name', 'deleted', 'visible' )

Expand Down Expand Up @@ -3374,6 +3383,7 @@ def to_hda_representative( self, multiple=False ):
return rval if multiple else rval[ 0 ]

def to_dict( self, view='collection' ):
original_dict_value = super(HistoryDatasetCollectionAssociation, self).to_dict( view=view )
dict_value = dict(
hid=self.hid,
history_id=self.history.id,
Expand All @@ -3382,6 +3392,9 @@ def to_dict( self, view='collection' ):
deleted=self.deleted,
**self._base_to_dict(view=view)
)

dict_value.update(original_dict_value)

return dict_value

def add_implicit_input_collection( self, name, history_dataset_collection ):
Expand Down Expand Up @@ -3418,7 +3431,7 @@ def copy( self, element_destination=None ):
return hdca


class LibraryDatasetCollectionAssociation( DatasetCollectionInstance, Dictifiable ):
class LibraryDatasetCollectionAssociation( DatasetCollectionInstance ):
""" Associates a DatasetCollection with a library folder. """
editable_keys = ( 'name', 'deleted' )

Expand Down Expand Up @@ -3615,7 +3628,7 @@ def __init__( self ):
self.user = None


class StoredWorkflow( object, Dictifiable):
class StoredWorkflow( HasTags, Dictifiable ):

dict_collection_visible_keys = ( 'id', 'name', 'published', 'deleted' )
dict_element_visible_keys = ( 'id', 'name', 'published', 'deleted' )
Expand All @@ -3637,13 +3650,6 @@ def copy_tags_from(self, target_user, source_workflow):

def to_dict( self, view='collection', value_mapper=None ):
rval = super( StoredWorkflow, self ).to_dict( view=view, value_mapper=value_mapper )
tags_str_list = []
for tag in self.tags:
tag_str = tag.user_tname
if tag.value is not None:
tag_str += ":" + tag.user_value
tags_str_list.append( tag_str )
rval['tags'] = tags_str_list
rval['latest_workflow_uuid'] = ( lambda uuid: str( uuid ) if self.latest_workflow.uuid else None )( self.latest_workflow.uuid )
return rval

Expand Down