DB 5.1__DEV11: unit support for time #3099

joshmoore · 2014-10-08T16:04:21Z

This commit introduces the ome.model.unit.Time class
which contains a double value and a string unit for
representing time durations. The only location which
makes use of this is PlaneInfo.exposureTime. "ms" is
hard-coded until the xsd-fu unit support is in place.

/cc @bpindelski @mtbc @qidane @rleigh-dundee

This commit introduces the ome.model.unit.Time class which contains a double value and a string unit for representing time durations. The only location which makes use of this is PlaneInfo.exposureTime. "ms" is hard-coded until the xsd-fu unit support is in place.

mtbc · 2014-10-08T20:32:44Z

Travis isn't green.

ghost · 2014-10-09T09:15:52Z

sql/psql/OMERO5.1DEV__11/schema.sql

@@ -1669,6 +1669,7 @@
        id int8 not null,
        deltaT float8,
        permissions int8 not null,
+        exposureTimeUnit varchar(255),


255 seems a bit wasteful; can't we use a much smaller size here given that most unit symbols and/or names will be very small?

Another possibility on the db side would be to have a foreign key reference to a time units table to make it restricted to the model enum values.

Possibly, but that's something we should do across the board in another PR, since we don't have any support for that at the moment. More interesting to me would be whether or not this should actually be an enum.

For any code to be able to robustly cope with units, I consider an enum as a necessity. This does constrain which units are possible to use of course, but the set Andrew has created covers all the SI system and then some, so I think the number of cases where this won't be sufficient is approaching zero.

Agreed, this was mostly just to have an example of touching all the necessary pieces. Just chatted through the current ome.xml classes that are needed:

class PlaneInfo { Time exposureTime; } class Time { double value; UnitsTime unit; } class UnitsTime { String symbol; String measurementSystem. }

Note: what I'd really like would be to have:

enum UnitsTime { }

but we don't currently have support for that in Hibernate and the DB. It might be worth it though.

joshmoore · 2014-10-10T07:06:29Z

Grrr.....

components/blitz/generated/omero/model/UnitsTime.ice:36: constant `UnitsTimeps' differs only in capitalization from constant `UnitsTimePs'

Also adds the TimeI implementation for the Python and C++ language bindings, including object factories and IceMapper code. The Java implementation is also an implementor of `ModelBased` so that IceMapper can convert back and forth to ome.model.units instances. For the next unit implementation (i.e. `Length`), it will likely be better to introduce code generation of the `ome.model.units` implementations rather than hand-writing them. Note: a workaround was needed for the generation of unique constant names in Ice since constants must vary in more than just capitalization which fails for SI prefixes like M and m.

joshmoore · 2014-10-14T10:46:42Z

--depends-on ome/bioformats#1379

joshmoore · 2014-10-14T10:47:43Z

Added dependency on the units work from @qidane, removed exclude and pushed the rest of the DB changes.

This commit adds an implicit dependency on the updated schema. Since there planeInfo.deltaT and pixels.timeIncrement have been modified to type Time, that takes place here as well. Note: this runs into an issue with hibernate (HHH-4329) in which the nice foreign key naming for embedded objects has to be removed. I'll attempt fixing that elsewhere. --depends-on ome/bioformats#1265

The few unit types are now corrected by hand since there is currently no way to code generate them. The map types were also missing properly named FKs, but these could be handled by the introduction of a new annotation. Note: a check has now been added to travis-build which should prevent generated FK constraint names from being included in a schema.

Java's FileReader and FileWriter use the default local encoding which led to some .ice files being created with latin1 encoding. Ice then understandably couldn't handle the micro symbol failing the build. See: http://stackoverflow.com/questions/6998905/java-bufferedwriter-object-with-utf-8

Ant's `<copy>` task uses the default encoding as well, meaning that the splitting of the combined file which DSLTask created with UTF-8 converted back to LATIN1 when the locale was C or similar. Further, charAt was not sufficient for detecting the micro symbol under C and so codePointAt is now used.

joshmoore · 2014-10-17T14:15:10Z

Restarted travis due to HDF5 weirdness.

For everyone's benefit, decision was to disable the tests for the moment (https://trac.openmicroscopy.org.uk/ome/ticket/12601) and look into this as a fix post m2.

melissalinkert · 2014-10-20T23:54:09Z

This and ome/bioformats#1379 are no longer in the breaking queue.

bpindelski · 2014-10-21T08:27:38Z

This broke web on trout with

InternalException at /webclient/load_data/dataset/7751/
exception ::omero::InternalException
{
    serverStackTrace = ome.conditions.InternalException:  Wrapped Exception: (org.springframework.orm.hibernate3.HibernateSystemException):
Null value was assigned to a property of primitive type setter of ome.model.units.Time.value; nested exception is org.hibernate.PropertyAccessException: Null value was assigned to a property of primitive type setter of ome.model.units.Time.value
    at org.springframework.orm.hibernate3.SessionFactoryUtils.convertHibernateAccessException(SessionFactoryUtils.java:679)

jburel · 2014-10-21T13:12:28Z

deleted my comment. I was using an out-of-date client

jburel · 2014-10-21T13:18:07Z

I did not notice the web error when starting the tests with an empty DB
I have:

Imported images
Browse dataset
Open the image in Viewer
Check Exposure time (see error below)

jburel · 2014-10-21T13:18:35Z

cc @will-moore

mtbc · 2014-10-21T14:05:58Z

If you can see what the upgrade script is failing to do, I am happy to try to patch it.

jburel · 2014-10-22T08:28:19Z

The issue is with the update script: so far tested upgrade from patch 10 to patch 11
The following error occurs when loading a dataset containing images imported prior to patch


Caused by: omero.InternalException
    serverStackTrace = "ome.conditions.InternalException:  Wrapped Exception: (org.springframework.orm.hibernate3.HibernateSystemException):
                        Null value was assigned to a property of primitive type setter of ome.model.units.Time.value; nested exception is org.hibernate.PropertyAccessException: Null value was assigned to a property of primitive type setter of ome.model.units.Time.value
                            at org.springframework.orm.hibernate3.SessionFactoryUtils.convertHibernateAccessException(SessionFactoryUtils.java:679)
                            at org.springframework.orm.hibernate3.HibernateAccessor.convertHibernateAccessException(HibernateAccessor.java:412)

No issue for images imported after applying the patch
This raises another issue: due to the way we use breaking (i.e. clean DB)
The upgrade scripts are never tested.

jburel · 2014-10-22T08:55:18Z

Pixels table:

images imported prior to the patch, have now a timeincrementunit value set to 11 (default second) even if the timeincrement value is null
but the newly imported images (after patch) have null for both

PlaneInfo:

deltat: all values are transformed to 11 i.e. id of seconds in unitstime.
deltatunit is alway null
- exposuretime is correctly preserved and exposuretimeunit correctly set to 11 (always) That might correct but will have to check later on with other images.

Error in the script for planeInfo

jburel · 2014-10-22T11:11:19Z

Closing this PR so it can move out of the breaking queue.
Replaced by gh-3129

joshmoore · 2014-10-27T13:19:53Z

Re-opening after closing gh-3129 so I can push a few more commits.

joshmoore · 2014-10-27T13:21:24Z

Note: @jburel's commits were tested by @mtbc in #3129

sbesson · 2014-10-28T12:18:03Z

Thanks all. Merging.

DB 5.1__DEV11: unit support for time

joshmoore · 2014-10-29T08:56:53Z

--no-rebase

joshmoore added 2 commits October 8, 2014 18:02

Rename SQL files with new schema version

9d689f1

ghost reviewed Oct 9, 2014
View reviewed changes

sbesson added the develop label Oct 11, 2014

cpp_obj_reg.vm: fix whitespace

98e7353

joshmoore force-pushed the dsl-units branch from 54df7e2 to 5d6b4d5 Compare October 14, 2014 08:49

joshmoore force-pushed the dsl-units branch from 5d6b4d5 to d586e43 Compare October 14, 2014 08:54

Improve Time documentation blocks

636f63b

joshmoore added the breaking label Oct 14, 2014

joshmoore force-pushed the dsl-units branch 2 times, most recently from 4d99186 to 48697b2 Compare October 14, 2014 11:17

joshmoore force-pushed the dsl-units branch from 48697b2 to e100a7a Compare October 14, 2014 11:21

joshmoore added 3 commits October 14, 2014 14:42

Upgrade scripts for DEV11

ecf8ee4

Minor test fix to deal with dep-graph issues

9d97e6c

joshmoore changed the title ~~DB 5.1__DEV11: unit support for PlaneInfo.exposureTime~~ DB 5.1__DEV11: unit support for time Oct 14, 2014

joshmoore force-pushed the dsl-units branch from 12a0404 to bdf7f3e Compare October 14, 2014 15:34

Fix grep test in travis-build

150866a

joshmoore force-pushed the dsl-units branch from bdf7f3e to 150866a Compare October 14, 2014 15:48

joshmoore added 2 commits October 14, 2014 20:20

melissalinkert removed the breaking label Oct 20, 2014

sbesson added the breaking label Oct 21, 2014

jburel added 3 commits October 22, 2014 10:10

deltatunit instead of deltat.

27799c5

Add where clause checking for non null value.

73e708a

Set correctly deltatunit and check for null.

fbd305b

jburel mentioned this pull request Oct 22, 2014

Dsl units #3129

Closed

jburel closed this Oct 22, 2014

jburel and others added 2 commits October 22, 2014 12:27

Fix the parentheses.

8d06dbd

Merge 'origin/develop' into dsl-units

d747f7e

joshmoore reopened this Oct 27, 2014

joshmoore removed the breaking label Oct 27, 2014

Re-add broken group to testImportScreenWithOnePlate

e0891b1

sbesson added a commit that referenced this pull request Oct 28, 2014

Merge pull request #3099 from joshmoore/dsl-units

a746e72

DB 5.1__DEV11: unit support for time

sbesson merged commit a746e72 into ome:develop Oct 28, 2014

joshmoore deleted the dsl-units branch October 29, 2014 08:56

sbesson added this to the 5.1.0-m2 milestone Nov 26, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DB 5.1__DEV11: unit support for time #3099

DB 5.1__DEV11: unit support for time #3099

joshmoore commented Oct 8, 2014

mtbc commented Oct 8, 2014

ghost Oct 9, 2014

ghost Oct 9, 2014

joshmoore Oct 9, 2014

ghost Oct 9, 2014

joshmoore Oct 9, 2014

joshmoore Oct 9, 2014

joshmoore commented Oct 10, 2014

joshmoore commented Oct 14, 2014

joshmoore commented Oct 14, 2014

joshmoore commented Oct 17, 2014

melissalinkert commented Oct 20, 2014

bpindelski commented Oct 21, 2014

jburel commented Oct 21, 2014

jburel commented Oct 21, 2014

jburel commented Oct 21, 2014

mtbc commented Oct 21, 2014

jburel commented Oct 22, 2014

jburel commented Oct 22, 2014

jburel commented Oct 22, 2014

joshmoore commented Oct 27, 2014

joshmoore commented Oct 27, 2014

sbesson commented Oct 28, 2014

joshmoore commented Oct 29, 2014

DB 5.1__DEV11: unit support for time #3099

DB 5.1__DEV11: unit support for time #3099

Conversation

joshmoore commented Oct 8, 2014

mtbc commented Oct 8, 2014

ghost Oct 9, 2014

Choose a reason for hiding this comment

ghost Oct 9, 2014

Choose a reason for hiding this comment

joshmoore Oct 9, 2014

Choose a reason for hiding this comment

ghost Oct 9, 2014

Choose a reason for hiding this comment

joshmoore Oct 9, 2014

Choose a reason for hiding this comment

joshmoore Oct 9, 2014

Choose a reason for hiding this comment

joshmoore commented Oct 10, 2014

joshmoore commented Oct 14, 2014

joshmoore commented Oct 14, 2014

joshmoore commented Oct 17, 2014

melissalinkert commented Oct 20, 2014

bpindelski commented Oct 21, 2014

jburel commented Oct 21, 2014

jburel commented Oct 21, 2014

jburel commented Oct 21, 2014

mtbc commented Oct 21, 2014

jburel commented Oct 22, 2014

jburel commented Oct 22, 2014

jburel commented Oct 22, 2014

joshmoore commented Oct 27, 2014

joshmoore commented Oct 27, 2014

sbesson commented Oct 28, 2014

joshmoore commented Oct 29, 2014