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

add support for multiple labels #111

Merged
merged 3 commits into from Aug 11, 2020
Merged

add support for multiple labels #111

merged 3 commits into from Aug 11, 2020

Conversation

ngreenwald
Copy link
Collaborator

This PR adds the functionality to handle different types of segmentation labels.

It changes the nomenclature to match how deepcell-toolbox deals with the labels (compartments). It removes hardcoding of dimension names that was previously used, and takes the name directly from whatever array is supplied. It also propagates this through the pipeline.

This closes #68

@ngreenwald ngreenwald requested a review from willgraf July 28, 2020 16:50
Copy link
Contributor

@willgraf willgraf left a comment

Choose a reason for hiding this comment

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

Just a couple questions regarding how we could possible reduce the number of hard-coded string values.

caliban_toolbox/utils/io_utils.py Outdated Show resolved Hide resolved
Comment on lines 223 to 228
dimension_labels = ['fovs', 'stacks', 'crops', 'slices', 'rows', 'cols', 'channels']
dimension_labels = ['fovs', 'stacks', 'crops', 'slices', 'rows', 'cols', 'compartments']
Copy link
Contributor

Choose a reason for hiding this comment

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

This list of strings is used twice, maybe we can move it to the top as a global/constant:
DIMENSION_LABELS = ['fovs', 'stacks', 'crops', 'slices', 'rows', 'cols', 'compartments']

@ngreenwald
Copy link
Collaborator Author

Is there a way to have project-wide global variables? I think the goal is to have consistent naming of the xarray dimensions across this whole repo, and have it match the nomenclature we're using in deepcell (hence the switch to compartments). Maybe if we define them as globals it will be more obvious that it's not some random hardcoded string? What do you think is the best way to handle that?

@willgraf
Copy link
Contributor

Yes, I often like to define a settings.py or config.py file for holding global variables / constants, and often use that file to parse environment variables.

@ngreenwald
Copy link
Collaborator Author

Do you have an example somewhere of how you format that and like to import it?

Copy link
Contributor

@willgraf willgraf left a comment

Choose a reason for hiding this comment

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

Looks much cleaner, I like it.

I left one comment regarding environment variable parsing; address that and I approve.

Comment on lines 32 to 38
X_DIMENSION_LABELS = config('DIMENSION_LABELS', cast=list,
default=['fovs', 'stacks', 'crops', 'slices',
'rows', 'cols', 'channels'])

Y_DIMENSION_LABELS = config('DIMENSION_LABELS', cast=list,
default=['fovs', 'stacks', 'crops', 'slices',
'rows', 'cols', 'compartments'])
Copy link
Contributor

Choose a reason for hiding this comment

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

How does casting to a list work? Does it expect a comma-separated entry or something?

@MekWarrior MekWarrior merged commit df42d68 into master Aug 11, 2020
@MekWarrior MekWarrior deleted the multiple_labels branch August 11, 2020 20:01
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

Successfully merging this pull request may close these issues.

add support for multiple label dimensions
4 participants