Tables new API by joshmoore · Pull Request #5481 · ome/openmicroscopy

joshmoore · 2017-08-31T11:04:28Z

What this PR does

Deprecate usage of the old PyTables API

Table creation and manipulation tested with the following version of pytables:

3.3.0 (Debian9)
3.2.x (Ubuntu16.04, CentOS 7)
3.1.x (ubuntu 14.04)

Testing this PR

check that the integration tests and training examples are green

Related reading

--depends-on #5409

In order to support old and new PyTables versions, this allows for choosing during service creation the storage provider. This is the first step in to allowing non-HDF backends.

dominikl · 2017-08-31T13:02:49Z

Will that also be available in the Java omero.grid.TablePrx ? Wanted to add some update methods to the gateway TablesFacility but couldn't find methods to add/delete rows/columns. incr and decr added with this PR seems to do that, right?

joshmoore · 2017-08-31T13:06:45Z

Will that also be available in the Java omero.grid.TablePrx ?

@dominikl : no, these are only backend changes.

add/delete rows/columns. incr and decr added with this PR seems to do that, right?

The methods weren't really added, only refactored from elsewhere. Their function is to maintain a counter of how often the file itself was opened, as opposed to column management. In general, PyTables doesn't easily provide column addition/removal. It requires recreating the file.

will-moore · 2017-09-05T10:35:00Z

Tests not passing yet... https://ci.openmicroscopy.org/view/Failing/job/OMERO-DEV-merge-integration-python/675/testReport/junit/OmeroPy.test.integration.tablestest.test_service/

joshmoore · 2017-09-05T10:41:48Z

merge-integration is still running.

mtbc · 2017-09-05T11:14:14Z

$ pip freeze | grep tables
tables==3.3.0

With Java integration tests on a Debian 9 system I am having some trouble with TableTest and TablesFacilityTest, though they passed fine on octopus. TestNG output is on squig in ome/team/mtbcarroll/PR-5481/index.html. Ping me if you'd like to log into the VM where this ran so you can check its config; I may have just screwed something simple up.

joshmoore · 2017-09-05T11:21:16Z

Thanks, @mtbc. Things I see:

AttributeError: 'Table' object has no attribute 'getWhereList - should be fixed here
AttributeError: 'Table' object has no attribute 'readCoordinates - should be fixed here
TypeError: shape must be an integer or sequence: 4L - perhaps PyTables 3.3.0 bug with Pandas DataFrame PyTables/PyTables#598 (comment), can you try upgrading, @mtbc ?

mtbc · 2017-09-05T11:42:57Z

Yes, if I upgrade to 3.4.2 then the TablesFacilityTest failure becomes another getWhereList one.

mtbc · 2017-09-05T11:53:18Z

(It's a pity that even Debian unstable is on tables 3.3.0 but I need to fiddle versions of other things anyway because Debian 9's packaged django stuff is too new for web.)

jburel · 2017-09-06T07:14:48Z

what are the issues with Django? Probably worth a card

mtbc · 2017-09-06T07:55:07Z

I don't know but have now created https://trello.com/c/GN2e5t9C/245-omeroweb-with-newer-django.

jburel · 2017-09-06T08:13:55Z

Comment added to card

mtbc · 2017-09-19T11:58:44Z

Taking a look at this, I've some confusion in that there are also omero::grid::Table methods getWhereList and readCoordinates in our API as listed at http://downloads.openmicroscopy.org/omero/5.4.0-m3/api/slice2html/omero/grid/Table.html and I doubt that I can rely on static checks to determine which kind of table is being called from where.

jburel · 2017-09-19T18:52:25Z

I have removed the exclude label.
Conflict will need to be fixed so it can be included in the build

TablesI now retries to acquire access to the related repository. omero.tables.RETRIES, by default 20, is used if not argument is passed to the constructor. Tests have been updated to perform no retries. See: https://trello.com/c/wPBnTfPU/58-flaky-tests

joshmoore · 2017-09-20T07:08:19Z

Cherry-picked the commit from gh-5512 since they were conflicting. I think this is ready for inclusion. (Though we may need to document non-working versions of PyTables. cc: @mtbc)

Usages of the two camel-cased methods were spread over hdfstoragev2 and columns. Splitting columns.py into 2 files as with the storage is both less useful (since no extension point is needed) and more disruptive since the file is already imported elsewhere. Instead, the few uses of camel-case have been wrapped with a check for the new version of PyTables and where necessary, the underscore version is used.

jburel · 2017-09-20T08:03:17Z

I added a comment while working on doc
see https://github.com/openmicroscopy/ome-documentation/pull/1739/files#diff-4f1690607b26288b56afc5164c52dce0
Do we want something else?

mtbc · 2017-09-20T12:15:41Z

===============================================
OmeroJava.integration
Total tests run: 2391, Failures: 0, Skips: 0
===============================================

BUILD SUCCESSFUL
Total time: 66 minutes 46 seconds
mtbc@mtbc-pr-5481:~/openmicroscopy$ pip freeze | grep tables
tables==3.4.2
mtbc@mtbc-pr-5481:~/openmicroscopy$ git log -1
commit 09a2aad8468d3d7ddee152938c2ea356aa506267
Merge: 1537f5a708 bb8a484334
Author: Mark Carroll <M.T.B.Carroll@Dundee.ac.uk>
Date:   Wed Sep 20 09:27:32 2017 +0000

    Merge remote-tracking branch 'origin/pr/5481' into HEAD
mtbc@mtbc-pr-5481:~/openmicroscopy$

Earlier issue now fixed. 👍

jburel · 2017-09-20T12:31:10Z

Did you install tables from pypi?
Have you tried with the "default" version on debian 9 (3.3)
I reckon it should work too

mtbc · 2017-09-20T12:35:32Z

I installed with pip install tables because I thought 3.3.0 would run into the TypeError at #5481 (comment) as at PyTables/PyTables#598 (comment).

joshmoore · 2017-09-20T12:38:41Z

The TypeError: shape must be an integer or sequence: 4L issue is definitely PyTables-specific and not related to this PR.

jburel · 2017-09-20T12:38:45Z

in this the case we will have to indicate that 3.3 is not working in the doc.
summary is: it works on 3.1, 3.2, 3.4 but not 3.3. (default on Debian 9)

This will require also an ome-install PR so it can be reflected in the installation doc

mtbc · 2017-09-20T12:48:35Z

Yes, to confirm,

mtbc@mtbc-pr-5481:~/openmicroscopy$ ./build.py -f components/tools/OmeroJava/build.xml test -DTEST=integration/gateway/TablesFacilityTest
...
                        TypeError: shape must be an integer or sequence: 4L
...
Exception in thread "TestNGInvoker-testAddTable()"
===============================================
Ant suite
Total tests run: 8, Failures: 1, Skips: 6
===============================================

mtbc@mtbc-pr-5481:~/openmicroscopy$ pip freeze | grep tables
tables==3.3.0
mtbc@mtbc-pr-5481:~/openmicroscopy$ dpkg --status python-tables | grep ^Version
Version: 3.3.0-5
mtbc@mtbc-pr-5481:~/openmicroscopy$

jburel · 2017-09-20T14:28:29Z

omero-install PR ome/omero-install#169

jburel · 2017-09-20T14:56:38Z

doc PR ome/omero-documentation#1771

jburel · 2017-09-20T20:50:52Z

i will check the tests tomorrow but it seems that it will be good to merge
I have opened two PR to fix the doc (installation and troubleshooting)

jburel · 2017-09-25T15:24:34Z

No table failure
merging

This adds on to 1264642 (omegh-5481) but not just adding a wait to the .omero/repository lookup but as well to the .omero/repository/*/repo_uuid lookup, so that the 20 REPEATS don't progress within 2-3 seconds but instead take long enough for the integration server has time to configure itself

@joshmoore

Adapts @joshmoore's metadata53 code while accommodating ome#5481

jburel and others added 7 commits August 1, 2017 13:52

Remove usage of old api

18b01a4

Merge remote-tracking branch 'origin/develop' into tables

7d6e9b4

Update method

b930de3

Allow to specify policy and handle error if strict

4e844a2

Specify policy in test

2e0734e

Merge remote-tracking branch 'origin/develop' into tables

f2e3faf

Support old and new PyTables in tests

e45214e

joshmoore mentioned this pull request Aug 31, 2017

table new api #5407

Closed

joshmoore changed the title ~~Tables legacy~~ Tables new API Aug 31, 2017

joshmoore and others added 4 commits August 31, 2017 14:23

Refactor HdfStorage out of tables.py

d946b50

In order to support old and new PyTables versions, this allows for choosing during service creation the storage provider. This is the first step in to allowing non-HDF backends.

Add logging of storage type

c87294a

Fix flake8 issue

6757278

Update unit tests

ca48b52

joshmoore force-pushed the tables-legacy branch from efc284b to ca48b52 Compare August 31, 2017 12:23

jburel added the develop label Sep 1, 2017

snoopycrimecop mentioned this pull request Sep 18, 2017

URL update #5511

Merged

jburel added the exclude label Sep 18, 2017

jburel removed the exclude label Sep 19, 2017

Merge branch 'develop' into tables-legacy

b8b15a1

snoopycrimecop mentioned this pull request Sep 20, 2017

tables: retry repository lookup #5512

Closed

joshmoore added 3 commits September 20, 2017 07:47

Update hdfstorageV1.py with https://trac

cd92e1b

Update hdfstorageV2.py with https://trac

841db19

tables: retry repository lookup

1264642

TablesI now retries to acquire access to the related repository. omero.tables.RETRIES, by default 20, is used if not argument is passed to the constructor. Tests have been updated to perform no retries. See: https://trello.com/c/wPBnTfPU/58-flaky-tests

joshmoore force-pushed the tables-legacy branch from 1ec5bc0 to 1eb824a Compare September 20, 2017 07:07

mtbc mentioned this pull request Sep 20, 2017

Version requirements ome/omero-documentation#1728

Merged

joshmoore force-pushed the tables-legacy branch from 1eb824a to bb8a484 Compare September 20, 2017 07:31

jburel mentioned this pull request Sep 20, 2017

Debian9 tables ome/omero-install#169

Merged

jburel merged commit a29c410 into ome:develop Sep 25, 2017

sbesson added this to the 5.4.0 milestone Oct 6, 2017

joshmoore mentioned this pull request Dec 6, 2017

tables: retry repo_uuid lookup #5593

Merged

mtbc added a commit to mtbc/openmicroscopy that referenced this pull request Mar 9, 2018

for HDF storage getOrCreate adjust file mode for read-only

9146f17

Adapts @joshmoore's metadata53 code while accommodating ome#5481

Conversation

joshmoore commented Aug 31, 2017

What this PR does

Testing this PR

Related reading

Uh oh!

dominikl commented Aug 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshmoore commented Aug 31, 2017

Uh oh!

will-moore commented Sep 5, 2017

Uh oh!

joshmoore commented Sep 5, 2017

Uh oh!

mtbc commented Sep 5, 2017

Uh oh!

joshmoore commented Sep 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtbc commented Sep 5, 2017

Uh oh!

mtbc commented Sep 5, 2017

Uh oh!

jburel commented Sep 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtbc commented Sep 6, 2017

Uh oh!

jburel commented Sep 6, 2017

Uh oh!

mtbc commented Sep 19, 2017

Uh oh!

jburel commented Sep 19, 2017

Uh oh!

joshmoore commented Sep 20, 2017

Uh oh!

jburel commented Sep 20, 2017

Uh oh!

mtbc commented Sep 20, 2017

Uh oh!

jburel commented Sep 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtbc commented Sep 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshmoore commented Sep 20, 2017

Uh oh!

jburel commented Sep 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtbc commented Sep 20, 2017

Uh oh!

jburel commented Sep 20, 2017

Uh oh!

jburel commented Sep 20, 2017

Uh oh!

jburel commented Sep 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jburel commented Sep 25, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dominikl commented Aug 31, 2017 •

edited

Loading

joshmoore commented Sep 5, 2017 •

edited

Loading

jburel commented Sep 6, 2017 •

edited

Loading

jburel commented Sep 20, 2017 •

edited

Loading

mtbc commented Sep 20, 2017 •

edited

Loading

jburel commented Sep 20, 2017 •

edited

Loading

jburel commented Sep 20, 2017 •

edited

Loading