Skip to content

Python gateway & webclient delete improvements#4266

Merged
sbesson merged 14 commits intoome:developfrom
ximenesuk:python-delete-improvements
Oct 21, 2015
Merged

Python gateway & webclient delete improvements#4266
sbesson merged 14 commits intoome:developfrom
ximenesuk:python-delete-improvements

Conversation

@ximenesuk
Copy link
Copy Markdown
Contributor

This PR addresses the various items in https://trello.com/c/WMnOBpC6/31-improve-deleteobjects-deprecate-deleteobjectdirect-inpython-gateway both RFEs and deprecation of deleteObjectDirect.

Tests, both OmeroPy and OmeroWeb should pass.

Web functionality to be tested is that which deletes and unlinks P/D/I, S/P and Annotations. I.e quite wide-ranging, but concentrating on unlinking and deleting all types of annotations would be useful.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can't see any direct use of this method, copyImageToDataset(). Should it be removed?

@ximenesuk
Copy link
Copy Markdown
Contributor Author

Closing temporarily

@ximenesuk ximenesuk closed this Oct 14, 2015
@ximenesuk ximenesuk reopened this Oct 18, 2015
@ximenesuk
Copy link
Copy Markdown
Contributor Author

DeleteObjects() doesn't take full advantage of the newer functionalities in Delete2 such as deleting different objects in one command. At some point a useful change might be to add something like a DeleteMultipleObjects() which takes a map of object types and ids.

@will-moore
Copy link
Copy Markdown
Member

Code changes seem to make sense on overview. Haven't tested functionality yet.

@ximenesuk
Copy link
Copy Markdown
Contributor Author

@sbesson the one thing I haven't done is mark deleteObjectsDirect() as deprecated. I was trying to recall if you had any luck with this or whether just adding a comment would have the same effect.

@joshmoore
Copy link
Copy Markdown
Member

A vote for DeprecationWarning:

$ git grep -l Deprec | grep Warning
# joshuas-mbp:ome0 jamoore$ git grep -n Deprec | grep Warning
components/tools/OmeroPy/src/omero/__init__.py:129:        category=DeprecationWarning)
components/tools/OmeroPy/src/omero/gateway/__init__.py:2767:            DeprecationWarning)
components/tools/OmeroPy/src/omero/gateway/__init__.py:8873:            "Deprecated. Use getImportedImageFiles()", DeprecationWarning)
components/tools/OmeroPy/src/omero/plugins/sessions.py:169:                    " instead.", DeprecationWarning)
components/tools/OmeroPy/src/omero/plugins/sessions.py:176:                    " instead.", DeprecationWarning)
components/tools/OmeroPy/src/omero/util/script_utils.py:1225:            "future versions of OMERO", DeprecationWarning)
components/tools/OmeroPy/src/omero/util/temp_files.py:140:                DeprecationWarning)

@ximenesuk
Copy link
Copy Markdown
Contributor Author

@joshmoore thanks. Clearly my git greping isn't up to scratch! I'll formally deprecate the method. (Especially as there is usage in the very same file!!)

@ximenesuk
Copy link
Copy Markdown
Contributor Author

In looking through the gateway I noticed that:
https://github.com/ximenesuk/openmicroscopy/blob/python-delete-improvements/components/tools/OmeroPy/src/omero/gateway/__init__.py#L7129
has been deprecated since 4.3.2, this would probably be a good time to remove this method.

@will-moore
Copy link
Copy Markdown
Member

Functionality of removing annotations in webclient works fine.
container.py certainly has some code that's not used now since the jsTree work, including move(), removemany() and copyImageToDataset() This will be removed as part of a full clean-up of this for 5.2.1.
Good to merge.

@joshmoore
Copy link
Copy Markdown
Member

joshmoore commented Oct 20, 2015 via email

@will-moore
Copy link
Copy Markdown
Member

@joshmoore I don't think that anything in the webclient can be considered an API (not like webgateway) - We don't deprecate or document changes to methods in webclient, we just go ahead and edit.

@joshmoore
Copy link
Copy Markdown
Member

Longer-term we should really re-evaluate that since it increases the burden on consumers. Or at least it needs to be made completely obvious, e.g. s/webclient/private/

@sbesson
Copy link
Copy Markdown
Member

sbesson commented Oct 20, 2015

The omeroweb.webclient module has certainly been exposed in the public Python API for every single release as well. I would vote for dropping the ambiguity as soon as possible.

@will-moore
Copy link
Copy Markdown
Member

@joshmoore @sbesson I would like to move all the "api" urls and view methods that power the jsTree etc into a new /url/ app, and add similar methods for other parts of the app as we go. Then mark everything remaining in webclient as non-public.

@ximenesuk
Copy link
Copy Markdown
Contributor Author

I hadn't realised that the getDataset was still being tested despite being deprecated since 4.3.2. Its use in the test has been removed in this final commit.

@ximenesuk
Copy link
Copy Markdown
Contributor Author

@sbesson
Copy link
Copy Markdown
Member

sbesson commented Oct 21, 2015

Given the state of https://ci.openmicroscopy.org/view/DEV/job/OMERO-DEV-merge-integration-python/ and https://ci.openmicroscopy.org/view/DEV/job/OMERO-DEV-merge-integration-Python27/, merging this. Let's have the webclient discussion separately. @will-moore: can you provide a listing of what is meant to be public and private within webclient?

sbesson added a commit that referenced this pull request Oct 21, 2015
Python gateway & webclient delete improvements
@sbesson sbesson merged commit 7a49e2d into ome:develop Oct 21, 2015
@will-moore
Copy link
Copy Markdown
Member

Currently @dpwrussell added a number of 'api' urls, that map to corresponding functions in views.py and tree.py that load the data via projection queries and marshal it to json:

api/groups/
api/experimenters/
api/experimenters/
api/containers/
api/datasets/
api/images/
api/share_images/
api/plates/
api/plate_acquisitions/
api/links/
api/paths_to_object/
api/tags/
api/shares/

These make a good starting point for public methods in webclient. More of what the webclient needs should be moved to this architecture and made public, probably in a new 'api' Django app.

@sbesson sbesson added this to the 5.2.0 milestone Oct 28, 2015
@sbesson sbesson mentioned this pull request Nov 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants