Skip to content

Comments

Organization updates#66

Open
emanuega wants to merge 16 commits intov0.1.7from
organization_updates
Open

Organization updates#66
emanuega wants to merge 16 commits intov0.1.7from
organization_updates

Conversation

@emanuega
Copy link
Owner

Allow raw images to be placed in a "Data" subdirectory and allow a data organization file to be included in the raw data directory. Allow the report path to specified and submit no report if no path is specified.

@pep8speaks
Copy link

pep8speaks commented Jun 14, 2020

Hello @emanuega! Thanks for updating this PR.

Line 132:38: E251 unexpected spaces around keyword / parameter equals
Line 132:40: E251 unexpected spaces around keyword / parameter equals
Line 132:53: E251 unexpected spaces around keyword / parameter equals
Line 132:55: E251 unexpected spaces around keyword / parameter equals

Line 75:13: W291 trailing whitespace
Line 210:44: E251 unexpected spaces around keyword / parameter equals
Line 210:46: E251 unexpected spaces around keyword / parameter equals
Line 210:81: E501 line too long (97 > 80 characters)
Line 222:71: W291 trailing whitespace

Comment last updated at 2020-11-24 02:48:31 UTC

@codecov
Copy link

codecov bot commented Jun 14, 2020

Codecov Report

Merging #66 (595136c) into v0.1.7 (8e2cb01) will decrease coverage by 18.65%.
The diff coverage is 85.18%.

Impacted file tree graph

@@             Coverage Diff             @@
##           v0.1.7      #66       +/-   ##
===========================================
- Coverage   87.33%   68.67%   -18.66%     
===========================================
  Files          57       57               
  Lines        5108     5140       +32     
===========================================
- Hits         4461     3530      -931     
- Misses        647     1610      +963     
Impacted Files Coverage Δ
merlin/analysis/generatemosaic.py 30.43% <ø> (-41.31%) ⬇️
merlin/merlin.py 67.88% <75.00%> (-6.40%) ⬇️
merlin/core/dataset.py 88.04% <80.00%> (-2.83%) ⬇️
merlin/analysis/optimize.py 33.87% <87.50%> (-41.81%) ⬇️
merlin/data/dataorganization.py 88.00% <88.23%> (-1.14%) ⬇️
merlin/util/dataportal.py 87.95% <100.00%> (+0.25%) ⬆️
merlin/plots/filterplots.py 25.73% <0.00%> (-74.27%) ⬇️
merlin/plots/decodeplots.py 26.90% <0.00%> (-68.43%) ⬇️
merlin/util/watershed.py 23.63% <0.00%> (-65.46%) ⬇️
merlin/util/decoding.py 12.14% <0.00%> (-64.29%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e2cb01...595136c. Read the comment docs.

emanuega and others added 6 commits June 14, 2020 13:30
@emanuega emanuega requested a review from seichhorn June 14, 2020 23:16
@emanuega emanuega changed the base branch from master to v0.1.7 June 14, 2020 23:17
Copy link
Collaborator

@seichhorn seichhorn left a comment

Choose a reason for hiding this comment

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

Looks good, couple points I was hoping you could address

stored in the dataSet, overwriting any previously stored
DataOrganization.
The DataOrganization is located in the following search order:
i) If filePath is specified and filePath exists this file is copied
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you change this to indicate to the user that it first looks for the filepath, if it doesn't find that it falls back to looking for the filepath prepended with merlin.DATA_ORGANIZATION_HOME. It also seems like we should issue a warning or error to the user if they have something in filepath that doesn't exist in either location, in case it is able to get a dataorganiation file from one of the other two if statements.

print('Running MERlin pipeline through snakemake')

if 'restart_times' not in snakemakeParameters:
snakemakeParameters['restart_times'] = 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

I worry that the typical user would not expect this behavior and might not understand why they have one job restarting itself. Could we log this somewhere the user might expect to find this type of information?


def __init__(self, dataSet, filePath: str = None):
def __init__(self, dataSet, filePath: str = None,
dataPortal: dataportal.DataPortal = None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should there be a way to add a dataportal when constructing an analysis folder with merlin.py?

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.

3 participants