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

OMERO.tables ArrayColumns #5632 (rebased onto develop) #728

Merged

Conversation

manics
Copy link
Member

@manics manics commented Feb 12, 2013

This is the same as gh-538 but rebased onto develop.


Add support for Long and Double arrays in the columns of OMERO.Tables, previously only atomic elements and masks were supported.

Integration tests have been updated.
Docs/examples will be dealt with in #9957
This should be 100% backwards compatible with the current API and existing tables.


Also incorporating rebased #753 (Trac #10403)
--rebased-to #753
--rebased-from #538

This needs reviewing by someone who knows about NumPy-Ice array conversions
…g a single recarray from the values and dtypes of each column in one go, instead of each co

lumn creating its own numpy.array
AbstractColumn.__init__ uses properties such as self.name which aren't always set at initialisation, especially when Ice is involved. This commit removes self.recarrtype and replaces it with just the properties that should exist (based on testing since its unclear under what circumstances Ice will set a property or not).
Mask creation and test functions also put into separate functions for reuse within test.
Test for a array column of size 1.
Test all possible column types in the same table.
Now that array columns seem to be working I can start to clean things up.
This replaces the need for a separate variable to indicate initialisation
This makes things logically nicer (new stuff goes at the end of the table)
Also moved HDF5 upload from class initialiser to the actual test method. This should make things easier if addition test files for future versions are needed.
…Table

The OriginalFile ID is useful for compatibility tests, as it allows the underlying file to be manually retrieved
@bpindelski
Copy link

Tested locally with Python 2.6. ./build.py -f components/tools/OmeroPy/build.xml test -DTEST=tablestest fails for the following test:

  • testAllColumns_4_4_5 ('bz2.BZ2File' object has no attribute '__exit__')

The baseline Python version we are currently supporting is 2.6, so the tests have to keep to 2.6 syntax.
/cc @joshmoore

@jburel
Copy link
Member

jburel commented Feb 14, 2013

@bpindelski: at the moment Python 2.5 is still the minimum requirement. even if the http://www.openmicroscopy.org/site/support/omero4/sysadmins/system-requirements.html
still indicates 2.4

@manics
Copy link
Member Author

manics commented Feb 15, 2013

@joshmoore
Copy link
Member

@manics: is your intent to fix 10403 in this PR?

@bpindelski
Copy link

@manics 👍 for the ticket. As this is a rebase, I'd vote for fixing it in a separate PR. Once it's open, we can merge this in.

@manics
Copy link
Member Author

manics commented Feb 15, 2013

@joshmoore It needs to go into dev_4_4 and develop, so separate PR.

@joshmoore
Copy link
Member

Agreed.

@manics
Copy link
Member Author

manics commented Feb 18, 2013

Now incorporating PR #753 (since AFAIK I can't open a PR which changes another PR).

@joshmoore
Copy link
Member

@manics: you can open a PR against your own PRs, but this is easily cleaner. Leave it for @bpindelski to give a thumbs up tomorrow.

@bpindelski
Copy link

@manics, @joshmoore This fixes the failing testAllColumns_4_4_5 test on develop. Looks good to merge.

@joshmoore
Copy link
Member

👍

joshmoore added a commit that referenced this pull request Feb 19, 2013
…n_5362-dev_4_4

OMERO.tables ArrayColumns #5632 (rebased onto develop)
@joshmoore joshmoore merged commit 30268ad into ome:develop Feb 19, 2013
@jburel
Copy link
Member

jburel commented Sep 10, 2013

--rebased-from #538
--rebased-to #753

@sbesson sbesson modified the milestones: 5.0.0, 5.0.0-alpha1 Nov 29, 2017
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

5 participants