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

add enumeration values for acquisition mode #4820

Merged
merged 4 commits into from Sep 23, 2016

Conversation

mtbc
Copy link
Member

@mtbc mtbc commented Sep 5, 2016

What this PR does

To the OMERO model object mapping adds acquisition modes,

  • BrightField
  • SweptFieldConfocal
  • SPIM

Testing this PR

The BIOFORMATS-DEV-merge-full-repository job's logs record various "Unknown AcquisitionMode value" warnings. Pick one of the corresponding images and import it into OMERO. Without these changes (including the entailed Bio-Formats changes) the Blitz log will warn that the enumeration does not exist and will set the image's acquisition mode to "Other". With this PR, having run the enums_update.sql script or used a build with #4837 included, the correct acquisition mode should be noted among the image's metadata for subsequent imports. (With these changes in OMERO but without running that database script import is as if OMERO hadn't been changed at all.)

Related reading

ome/bioformats#2553
https://trello.com/c/0zNp8HvU/167-enumerations-addition

@sbesson
Copy link
Member

sbesson commented Sep 7, 2016

Tested using the breaking trout server (which included both this PR and the corresponding Bio-Formats one).

Tested with the following workflow:

  • the server DB was generated by the CI job from an existing 5.2__0 reference DB and upgraded via the SQL script using omego db upgrade
  • import of a JP2 file with an existing LaserScanningConfocalMicroscopy value for AcquisitionMode:

screen shot 2016-09-07 at 13 42 55

  • import of a CZI file with a new SPIM value for AcquisitionMode:

screen shot 2016-09-07 at 13 48 55

Here the new enum is recognized by Bio-FOrmats (as shown using the command line tools) but the DB enum has not been updated in the DB as there is no DB patch script for now so the value is converted to Other

  • ran psql OMERO-DEV-breaking-deploy -f sql/psql/OMERO5.3DEV__11/enums_update.sql
  • import 4-3-3 fixes. #2 of a CZI file with a SPIM value for AcquisitionMode value (new enum):

screen shot 2016-09-07 at 13 51 53

In summary, the logic works as expected when using a version patch of the schema and updating the DB using the enums-update.sql. Some questions have been raised about the enums ordering on the Java side to minimize breakages. From the OMERO standpoint and the ability to make non-breaking enums addition to the schema and the DB, this raises the question of how the DB enums should be handled. Should we include the enums-update.sql script in the official upgrade workflow or even as a server startup task? /cc @joshmoore @jburel

@mtbc
Copy link
Member Author

mtbc commented Sep 16, 2016

To test the ABI compatibility issue raised by @rleigh-codelibre:

  1. first

    a. populate database from scripts generated without this PR
    b. use enums_update.sql from this PR
    c. note which string in the acquisitionmode table gets which ID

  2. then

    a. with this PR, run bin/omero db script
    b. populate the database from that
    c. note which string in the acquisitionmode table gets which ID

  3. compare the outcome of the two previous steps to make sure the same strings got the same IDs.

@will-moore
Copy link
Member

With this DB:

Started with old DB:

omero44=# select * from dbpatch;
 id | currentpatch | currentversion | permissions |          finished          |      message      | previouspatch | previousversion | external_id 
----+--------------+----------------+-------------+----------------------------+-------------------+---------------+-----------------+-------------
  1 |            0 | OMERO4.4       |         -52 | 2016-09-07 10:10:58.774547 | Database ready.   |             0 | OMERO4.4        |            
  2 |            0 | OMERO5.0       |         -52 | 2016-09-07 10:44:04.595493 | Database updated. |             0 | OMERO4.4        |            
  3 |            1 | OMERO5.1       |         -52 | 2016-09-07 11:57:35.467874 | Database updated. |             0 | OMERO5.0        |            
  4 |            0 | OMERO5.2       |         -52 | 2016-09-07 12:28:13.696795 | Database updated. |             1 | OMERO5.1        |            
  5 |           11 | OMERO5.3DEV    |         -52 | 2016-09-07 12:28:51.174182 | Database updated. |             0 | OMERO5.2        |        

omero44=# select * from acquisitionmode;
 id | permissions |                value                | external_id 
----+-------------+-------------------------------------+-------------
  1 |         -52 | WideField                           |            
  2 |         -52 | LaserScanningConfocalMicroscopy     |            
  3 |         -52 | SpinningDiskConfocal                |            
  4 |         -52 | SlitScanConfocal                    |            
  5 |         -52 | MultiPhotonMicroscopy               |            
  6 |         -52 | StructuredIllumination              |            
  7 |         -52 | SingleMoleculeImaging               |            
  8 |         -52 | TotalInternalReflection             |            
  9 |         -52 | FluorescenceLifetime                |            
 10 |         -52 | SpectralImaging                     |            
 11 |         -52 | FluorescenceCorrelationSpectroscopy |            
 12 |         -52 | NearFieldScanningOpticalMicroscopy  |            
 13 |         -52 | SecondHarmonicGenerationImaging     |            
 14 |         -52 | PALM                                |            
 15 |         -52 | STORM                               |            
 16 |         -52 | STED                                |            
 17 |         -52 | TIRF                                |            
 18 |         -52 | FSM                                 |            
 19 |         -52 | LCM                                 |            
 20 |         -52 | Other                               |            
 21 |         -52 | Unknown                             |            
(21 rows)

Ran sql/psql/OMERO5.3DEV__10/enums_update.sql

DB updated to:

omero44=# select * from acquisitionmode;
 id | permissions |                value                | external_id 
----+-------------+-------------------------------------+-------------
  1 |         -52 | WideField                           |            
  2 |         -52 | LaserScanningConfocalMicroscopy     |            
  3 |         -52 | SpinningDiskConfocal                |            
  4 |         -52 | SlitScanConfocal                    |            
  5 |         -52 | MultiPhotonMicroscopy               |            
  6 |         -52 | StructuredIllumination              |            
  7 |         -52 | SingleMoleculeImaging               |            
  8 |         -52 | TotalInternalReflection             |            
  9 |         -52 | FluorescenceLifetime                |            
 10 |         -52 | SpectralImaging                     |            
 11 |         -52 | FluorescenceCorrelationSpectroscopy |            
 12 |         -52 | NearFieldScanningOpticalMicroscopy  |            
 13 |         -52 | SecondHarmonicGenerationImaging     |            
 14 |         -52 | PALM                                |            
 15 |         -52 | STORM                               |            
 16 |         -52 | STED                                |            
 17 |         -52 | TIRF                                |            
 18 |         -52 | FSM                                 |            
 19 |         -52 | LCM                                 |            
 20 |         -52 | Other                               |            
 21 |         -52 | Unknown                             |            
 22 |         -35 | BrightField                         |            
 23 |         -35 | SweptFieldConfocal                  |            
 24 |         -35 | SPIM                                |            
(24 rows)

@will-moore
Copy link
Member

Testing $ omero db script.

$ psql -U omero omero4820 < OMERO5.3DEV__11.sql 
...
omero4820=# select * from acquisitionmode;
 id | permissions |                value                | external_id 
----+-------------+-------------------------------------+-------------
  1 |         -52 | WideField                           |            
  2 |         -52 | LaserScanningConfocalMicroscopy     |            
  3 |         -52 | SpinningDiskConfocal                |            
  4 |         -52 | SlitScanConfocal                    |            
  5 |         -52 | MultiPhotonMicroscopy               |            
  6 |         -52 | StructuredIllumination              |            
  7 |         -52 | SingleMoleculeImaging               |            
  8 |         -52 | TotalInternalReflection             |            
  9 |         -52 | FluorescenceLifetime                |            
 10 |         -52 | SpectralImaging                     |            
 11 |         -52 | FluorescenceCorrelationSpectroscopy |            
 12 |         -52 | NearFieldScanningOpticalMicroscopy  |            
 13 |         -52 | SecondHarmonicGenerationImaging     |            
 14 |         -52 | PALM                                |            
 15 |         -52 | STORM                               |            
 16 |         -52 | STED                                |            
 17 |         -52 | TIRF                                |            
 18 |         -52 | FSM                                 |            
 19 |         -52 | LCM                                 |            
 20 |         -52 | Other                               |            
 21 |         -52 | Unknown                             |            
 22 |         -52 | BrightField                         |            
 23 |         -52 | SweptFieldConfocal                  |            
 24 |         -52 | SPIM                                |            
(24 rows)

Looks to be the same as updated DB above.

@sbesson
Copy link
Member

sbesson commented Sep 23, 2016

As discussed with @mtbc, merging.

@sbesson sbesson merged commit 2d2ba8f into ome:develop Sep 23, 2016
@mtbc mtbc deleted the add-enums-minimal branch September 23, 2016 07:52
@jburel jburel mentioned this pull request Oct 14, 2016
@jburel jburel added this to the 5.3.0 milestone Mar 29, 2017
rgozim pushed a commit to rgozim/openmicroscopy that referenced this pull request Oct 8, 2018
add enumeration values for acquisition mode
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.

None yet

4 participants