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

Introduce labels for all modules, remove name handling field for data inputs #3644

Merged
merged 28 commits into from Mar 1, 2017

Conversation

Projects
None yet
4 participants
@guerler
Copy link
Contributor

commented Feb 21, 2017

This PR introduces unified label and annotation handling for all workflow modules. Additionally, it hides the name input field for data input parameters and transfers the value upon workflow loading to the label attribute. This way backward compatibility for existing workflows should be ensured.

@guerler guerler added this to the 17.05 milestone Feb 21, 2017

@guerler guerler changed the title Introduce labels for all modules, disable name handling field for data inputs Introduce labels for all modules, hide name handling field for data inputs Feb 21, 2017

guerler added some commits Feb 22, 2017

@guerler guerler force-pushed the guerler:unify_label_name_000 branch from 964ff10 to f652e9c Feb 22, 2017

@guerler guerler changed the title Introduce labels for all modules, hide name handling field for data inputs Introduce labels for all modules, remove name handling field for data inputs Feb 22, 2017

guerler added some commits Feb 22, 2017

if not module.label and module.type in [ 'data_input', 'data_collection_input' ] and isinstance( state, dict ):
default_label = state.get( 'name' )
if str( default_label ).lower() != module.name.lower():
module.label = default_label

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Feb 22, 2017

Member

It is not clearly to me why you compare default_label with module.name, but then change module.label if they are different.

This comment has been minimized.

Copy link
@guerler

guerler Feb 22, 2017

Author Contributor

The reason is that the name value was previously initialized with the default_name and stored in the module state. In case of data_input the default_name was Input Dataset, and for data_collection_input it was Input Dataset Collection. These default names are not-unique and should not be transferred to the label attribute. If transferred they would cause problems when a user loads an existing workflow and attempts to save it i.e. duplicate label error modal would appear until all labels are unique. Now in order to avoid confusion, these default values are identified and filtered by using the constant module.name attribute which is set to Input dataset and Input dataset collection.

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Feb 22, 2017

Member

Thanks for the explanation @guerler! Would it make sense then to just replace:

if str( default_label ).lower() != module.name.lower():

with:

if str( default_label ).lower() not in ['input dataset', 'input dataset collection']:

?

This comment has been minimized.

Copy link
@guerler

guerler Feb 22, 2017

Author Contributor

Yes, it would and its much better to do that, otherwise it might cause issues if someone decides to change the fixed module name. Thanks a lot.

@guerler guerler force-pushed the guerler:unify_label_name_000 branch from cd5fef4 to b9ba754 Feb 24, 2017

@guerler guerler requested a review from jmchilton Feb 24, 2017

@guerler guerler added status/review and removed status/WIP labels Feb 26, 2017

@martenson martenson merged commit d3840c5 into galaxyproject:dev Mar 1, 2017

4 checks passed

api test Build finished. 263 tests run, 0 skipped, 0 failed.
Details
framework test Build finished. 140 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 24 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 580 tests run, 0 skipped, 0 failed.
Details
@martenson

This comment has been minimized.

Copy link
Member

commented Mar 1, 2017

Thanks @guerler !

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.