-
Notifications
You must be signed in to change notification settings - Fork 24
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 the GrabCut example #193
base: main
Are you sure you want to change the base?
Conversation
@jlstevens if you don't mind having a look at this, I think that this time we're closer to what we wanted to have from the beginning! Too bad that the annotators seem to be broken in geoviews, I just realized it quite late in the process of updating the main grabcut class. |
That's alarming!! |
Thanks for the PR! I'll try this out tonight - this will be a nice test case for my deployment tool. :-)
That sounds like an issue to file on geoviews if you can submit a small, reproducible example... |
@jlstevens thanks! And there was already an issue opened on geoviews 🙃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Co-authored-by: James A. Bednar <jbednar@users.noreply.github.com>
cb32fc0
to
996590f
Compare
I've rebased this PR and if I can get it to build I'll merge to master and update the public website. |
@maximlt just reminded me that there is one last change to be made to the grabcut project: we need to serve a panel dashboard with |
@jlstevens I've made Grabcut servable by creating a small dashboard with a pipeline that includes the first two stages introduced in the notebook. I've not added the 3rd stage (allow to manually edit the contour obtained after running grabcut) since the 4th stage which should allow to download the output (or do something with it, anything really) isn't included in the notebook, since it wasn't included in Earthsim. I don't even know if it's possible to export a Path or Polygon to a shapefile with geoviews? |
Looks great! A two stage pipeline is fine for now: I'll see if I can deploy a running version of this project shortly and if that works well, we can think about whether it is worth introducing more stages. |
Cool! I believe shapely can export a shapefile. |
That would be with |
Oh man, I can definitely understand why Maxime had concerns about panel-chat-examples and future maintenance; for old, unmaintained repos, its a nightmare. Took me the whole day to get it barely functioning. It's cool when it works, but super, extremely finicky. Area.mp4This drained the life out of me, and I'd prefer to rebuild existing ideas thru slightly more modern tools like https://twitter.com/giswqs/status/1649416858645282822 instead next time. |
grabcut/grabcut.py
Outdated
@@ -320,8 +320,8 @@ def view(self): | |||
dmap = hv.DynamicMap(self.extract_foreground) | |||
dmap = hv.util.Dynamic(dmap, operation=self._filter_contours) | |||
dmap = hv.util.Dynamic(dmap, operation=self._simplify_contours) | |||
return (regrid(self.image).options(**options) * self.bg_paths * self.fg_paths + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was the regrid add here? It kept making the shapes mismatched and took me forever to find what caused the shape mismatches :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably to make it possible to display really large images without causing issues in the browser? That would be my initial guess at least.
Welcome to the gang :D Code that doesn't run dies quicker than one would think! |
Would like a review on this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! No longer any outlandish imports!
grabcut/grabcut.ipynb
Outdated
"\n", | ||
"Written by Philipp Rudiger and Maxime Liquet<br>\n", | ||
"Created: November 16, 2018<br>\n", | ||
"Last updated: January 7, 2021" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the dates auto-update on merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I don't think so.
Can we merge this? |
Please! |
Let me review please, I see some changes to make, to the project config for instance... |
@ahuang11 did you use anaconda-project to set up your environment to work on Grabcut? I see the project is not locked on |
It's been a while so I don't really remember. I believe I did, but I didn't lock the project. |
This PR adds the GrabCut example taken from Earthsim's topics. While the original example relied on the
earthsim
library, this example relies on the moduleearthsim.py
that sits next to the notebook. This module has been extracted and adapted fromearthsim
. Doing so has allowed to significantly reduce the number of dependencies required to run the grabcut example, and as a consequence made it possible to update the remaining ones.Compared to the original example:
quest
to download map tiles, instead the module relies on the functionget_tile_rgb
available in theutil.py
module of geoviews.PolyAnnotator
to modify the contour generated during the main/previous stage has been entirely removed since annotators are currently broken in geoviews (annotators wont work in geoviews holoviz/geoviews#526).