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

BlockMethod to run any SIMA segmentation strategy #168

Merged
merged 2 commits into from
Apr 13, 2015

Conversation

pkaifosh
Copy link
Contributor

I've created a simple BlockMethod/BlockAlgorithm pair that should allow any sima segmentation strategy to be run on a thunder block. Included is a single test case, which only runs if sima is installed. The test case passes on my laptop.

On a related note, I may try to simplify the dependencies of SIMA a bit so that it might be made a dependency of thunder without much pain. The current dependencies are:

Python 2.7
numpy >= 1.6.2
scipy >= 0.13.0
scikit-image >= 0.9.3
scikit-learn >= 0.11
shapely >= 1.2.14
pillow >= 2.6.1
future

@freeman-lab Let me know which ones you think would have to best converted to optional dependencies prior to making sima a thunder dependency.


Parameters
----------
sima_strategy : sima.segment.SegmentationStrategy
Copy link
Member

Choose a reason for hiding this comment

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

Change sima_strategy in docstring to match simaStrategy as below

@freeman-lab
Copy link
Member

This looks great @pkaifosh , thanks! Only left a couple minor formatting requests.

Regarding making it a dependency, it looks like it will be quite smooth now, thanks for the effort! All of the ones you list are also dependencies for thunder except future and shapely. Adding future seems fine, and I had been thinking about adding shapely anyway. So far every time I've considered it I found I could do similar things with matplotlib or skimage, though I imagine that won't always be true. Just out of curiosity, where have you found shapely to be crucial?

Why don't we plan to merge your PR as it is now (with SIMA as an optional dependency), then I can make it a full dependency once I've done some testing to make sure everything installs fine on our EC2 builds. Sounds good?

@pkaifosh
Copy link
Contributor Author

Thanks for the feedback, @freeman-lab . I'll make the changes that you have suggested, and then you can merge the PR.

Regarding shapely, we use it for the polygon representations of our ROIs. I doubt it is crucial, as we use only very limited functionality from it, and I really wouldn't mind removing the dependency since it has caused installation headaches for some of our Windows users due to DLL issues. We just haven't got around to substituting it out though because, other than the installation issues on Windows, it works as is, and there are so many other things to do.

@freeman-lab
Copy link
Member

Awesome, this LGMT @pkaifosh ! Thanks for the contribution! First merged PR from CodeNeuro =)

And maybe I can help with that shapely dependency, I think we're using a scipy function for getting polygons from masks (but looked at something similar in shapely), maybe that's all you need too, I can take a look.

freeman-lab added a commit that referenced this pull request Apr 13, 2015
BlockMethod to run any SIMA segmentation strategy
@freeman-lab freeman-lab merged commit 057c25c into thunder-project:master Apr 13, 2015
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.

None yet

2 participants