Fixed the issues with the tests in Eric's roi branch #306
Conversation
|
@ericdill tests are ok, issue with pandas |
|
fixed the issue |
|
I am 👎 on including |
|
@ericdill I took it out, now ready for review |
| @@ -22,6 +22,7 @@ install: | |||
| - python setup.py install | |||
| - pip install coveralls | |||
| - pip install codecov | |||
| - pip install pandas | |||
There was a problem hiding this comment.
Can you conda install pandas instead? (Add it to line 18)
|
@ericdill I took out the pandas dependency in these test, and rebased this PR. Ready for review. |
| labeled array; 0 is background. | ||
| Each ROI is represented by a distinct label (i.e., integer). | ||
|
|
||
| mean_intensity : list |
There was a problem hiding this comment.
This is coming back an as array not a list
There was a problem hiding this comment.
Fixed the issue of mean_intensity coming as array, it returns a list
There was a problem hiding this comment.
I would have just changed the documentation. Arrays are far more useful than lists of lists
There was a problem hiding this comment.
Yeah, I'm with Tom on this one. Can you undo the change to the code and update the docs to say that it is coming back as an array?
There was a problem hiding this comment.
Sure I will make it to return an array instead of a list
it now takes a list of bad data that should be removed before data is returned from the function as a list of rois
It now returns a dataframe
Using pandas drop row/col functions instead
Used to return np.matrix, now returns np.vstack
We should test the public function that uses the private function, not the private function itself.
Conflicts: .travis.yml
@danielballan made a convincing argument that the library functions of skxray should not return DataFrames. If we want/need DataFrames, then fancy callable classes could be used to do this, but those callable classes still need to call library functions to do all their work Conflicts: .travis.yml conda-recipe/meta.yaml
added new function to get combine mean intensities for image sets
BUG: Conflicts: .travis.yml
Conflicts: .travis.yml
…ing strings on the mean intensity function
|
Fixed the issues ready for review |
| @@ -10,14 +10,17 @@ before_install: | |||
| - chmod +x miniconda.sh | |||
| - ./miniconda.sh -b -p /home/travis/mc | |||
| - export PATH=/home/travis/mc/bin:$PATH | |||
| - wget https://gist.githubusercontent.com/tacaswell/128bb482f845feb024eb/raw/5cf21dc03a354fc87140d4a75e17cb5c076a0517/.condarc -O /home/travis/.condarc | |||
There was a problem hiding this comment.
why is this being added back in to the travis yaml?
There was a problem hiding this comment.
It is a mistake I will take it out in travis yaml
…lier it was returning a list)
|
Fixed ready for review |
| # turn the 2-D array back into a list of arrays because the rows and | ||
| # columns have different units | ||
| mean_intensity = [data[:, i] for i in range(data.shape[1])] | ||
| return np.asarray(mean_intensity), index |
There was a problem hiding this comment.
It is entirely unclear why you are laundering this numpy array through a list and back to a numpy array...
|
@ericdill now the mean_intensity only return as a bumpy array, took out the list |
| except TypeError: | ||
| index = [index] | ||
| # not sure that this is needed | ||
| index = np.asarray(index) |
There was a problem hiding this comment.
This line is not needed. The index argument to ndim.mean just needs to be a sequence, not a numpy array
|
Great, thanks for all the work! |
Add functions for kymograph and ROI aware mean computation for image stack
|
Sure, Thank you for all the help! |
No description provided.