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

V0.3 qp workflow #6

Merged
merged 8 commits into from
Nov 8, 2021
Merged

V0.3 qp workflow #6

merged 8 commits into from
Nov 8, 2021

Conversation

pwalczysko
Copy link
Member

@pwalczysko pwalczysko commented Oct 18, 2021

In preparation of the Turku workshop, adding new features to the guide.

Ready for review.

cc @jburel

@pwalczysko pwalczysko changed the title WiP: V0.3 qp workflow V0.3 qp workflow Nov 2, 2021
@pwalczysko
Copy link
Member Author

Ready for review cc @melvingelbard

@melvingelbard
Copy link

Looks good to me!
Maybe it could directly link to the right section of the QuPath docs to refer back to it to avoid confusion (e.g.
https://qupath.readthedocs.io/en/latest/docs/advanced/omero.html#browsing-an-omero-server & https://qupath.readthedocs.io/en/latest/docs/advanced/omero.html#send-objects-back-to-your-omero-server).

I was a bit confused at the bit where it refers to Detections as 'derived Annotations'. Annotation and Detection objects are very similar in some ways but quite different in the way they are processed in QuPath. But I'm not sure this is very important in this case :)

It's very nice to see tutorials on this. Looking forward to working more on the extension and listening to more feedback for improvement!

docs/qupath.rst Outdated

Save detection ROIs using ome-omero-roitool
-------------------------------------------
This workflow necessritates the usage of the Command Line Interface. The limitation here are the Annotation ROIs, whcih are transformed into masks in OMERO. Although this preserves the holes in the Annotations, if the Annotation ROIs are too large, it might result in performance problems or even running out of resources on the machine where the export of the mask from QuPath is attempted.
Copy link
Member

Choose a reason for hiding this comment

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

typo "necessritates"

Copy link
Member

Choose a reason for hiding this comment

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

and "whcih"

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 00853dd

@will-moore
Copy link
Member

Looks good (at https://omero-guides--6.org.readthedocs.build/projects/qupath/en/6/qupath.html 👍 ).
Few minor comments. NB: I haven't tried running the steps myself.

@pwalczysko
Copy link
Member Author

fixes pushed e7fc3cc, 7e49f72 cc @melvingelbard - Re the derived Annotations formulation - I have removed this formulation replacing it with derrived ROIs. I have left at another place the Annotation ROIs though - let me know if this is acceptable. The problem we are having here in terms of terminology is:

In OMERO, all ROIs are a subtype of annotaion, other annotations might be e.g. key-value pairs, or Tags etc.
In QuPath, afaik, the Annotations are what we would call in OMERO (for the lack of better terms) "special large ROIs". I would like to avoid this explanation in the workflow, as it is bound to be confusing.

@pwalczysko
Copy link
Member Author

@will-moore fix pushed 00853dd

@will-moore
Copy link
Member

Looks great, thanks.

@pwalczysko
Copy link
Member Author

@will-moore thank you. @jburel can we merge please so that we are ready for next week's outreach ?

@jburel jburel merged commit bf4bfba into ome:master Nov 8, 2021
@jburel
Copy link
Member

jburel commented Nov 8, 2021

@pwalczysko I have adjusted the permissions on that repo, so that next time you can merge

@pwalczysko pwalczysko deleted the v0.3-QP-workflow branch November 8, 2021 14:20
@pwalczysko
Copy link
Member Author

@pwalczysko I have adjusted the permissions on that repo, so that next time you can merge

@jburel thank you

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.

4 participants