Scalebar units - Githubissues

will-moore commented 4 years ago

Allow user to choose units for scalebar - Fixes #331

Testing can use images with various pixel sizes created at https://merge-ci.openmicroscopy.org/web/webclient/?show=dataset-14769 (created with this script: https://gist.github.com/will-moore/68cccecf9646571fa1d96a981eac0cce)

To test:

WITHOUT this PR, create a figure with containing images with various pixel sizes (see Dataset above),
- Add scalebars with labels, using same Units as the pixel size units.
- Leave 1 or 2 images without scalebars.
- Export to PDF/TIFF to have a record of what it looked like.
Update to use this PR - open the figure created earlier and test that scalebars look the same.
Change the units on existing scalebars - they should update as expected.
Add new images and add scalebars to these and to older images without scalebars: Default scalebars should be created in the same units as pixel sizes.
Try changing units for scalebar... And show label...
Label should update & scalebar should be expected length
Test export as TIFF and PDF

will-moore commented 4 years ago

Travis failing with

>       assert script_service.getParams(id) is not None
test/integration/test_figure_scripts.py:51: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = bd3f8e82-c1dd-41e6-bc2a-672ba7829577/93467715-93ba-4fae-8967-fe8409c15f4comero.api.IScript -t -e 1.1:tcp -h 172.18.0.4 -p 35703 -t 60000
scriptID = 63, _ctx = None
    def getParams(self, scriptID, _ctx=None):
>       return _M_omero.api.IScript._op_getParams.invoke(self, ((scriptID, ), _ctx))
E       omero.InternalException: exception ::omero::InternalException
E       {
E           serverStackTrace = 
E           serverExceptionClass = 
E           message = Sha1s don't match! expected 4cfaf0e9cbf68fb5dc25bc2816f8c3b83a7248e7, found cd16eafce5be6f5f47bf66ce78d6ac730115ce88
E       }

manics commented 4 years ago

Maybe a race condition along the lines of the scripts are copied in but OMERO hasn't yet detected they've been modified?

will-moore commented 4 years ago

Running locally with that last commit...

+ su omero-server
+ omero login -w omero root@omero
FATAL: Running /opt/omero/server/OMERO.server/bin/omero as root can corrupt your directory permissions.

The su omero-server seems to be ignored since omero login ... still acts as if the root user is running it?

jburel commented 4 years ago

if you have a .env file exporting path and omerodir have you tried su - omero-server -c ". path/settings.env omero login ...."

will-moore commented 4 years ago

With that last commit, locally I get:

+ su - omero-server -c 'env OMERODIR=/opt/omero/server/OMERO.server /opt/omero/server/OMERO.server/bin/omero script upload /omero-figure/omero_figure/scripts/omero/figure_scripts/Figure_To_Pdf.py --official'
Using session for root@omero:4064. Idle timeout: 10 min. Current group: system
Traceback (most recent call last):
  File "/opt/omero/server/OMERO.server/bin/omero", line 131, in <module>
    rv = omero.cli.argv()
  File "/opt/omero/server/venv3/lib64/python3.6/site-packages/omero/cli.py", line 1756, in argv
    cli.invoke(args[1:])
  File "/opt/omero/server/venv3/lib64/python3.6/site-packages/omero/cli.py", line 1187, in invoke
    stop = self.onecmd(line, previous_args)
  File "/opt/omero/server/venv3/lib64/python3.6/site-packages/omero/cli.py", line 1264, in onecmd
    self.execute(line, previous_args)
  File "/opt/omero/server/venv3/lib64/python3.6/site-packages/omero/cli.py", line 1346, in execute
    args.func(args)
  File "/opt/omero/server/venv3/lib/python3.6/site-packages/omero/plugins/script.py", line 651, in upload
    id = scriptSvc.uploadOfficialScript(args.file, p.text())
  File "/opt/omero/server/venv3/lib64/python3.6/site-packages/path.py", line 814, in text
    return f.read()
  File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 47015: ordinal not in range(128)

will-moore commented 4 years ago

@joshmoore @manics @jburel - Any ideas how to fix upload of the script that now has unicode characters in it (see error above). NB: you don't see this in the travis failure logs.

will-moore commented 4 years ago

I see the same error if I manually try to upload the script...

$ NOCLEAN=true .omero/docker app
...
$ .omero/compose exec web bash

bash-4.2$ cd /opt/omero/web/OMERO.web/
bash-4.2$ bin/omero login
Server: [localhost:4064]omero:4064
Username: [omero-web]root
Password:     # omero

bash-4.2$ cd omero_figure/scripts/
bash-4.2$ /opt/omero/web/OMERO.web/bin/omero script upload omero/figure_scripts/Figure_To_Pdf.py --official

Using session for root@omero:4064. Idle timeout: 10 min. Current group: system
Traceback (most recent call last):
  File "/opt/omero/web/venv3/bin/omero", line 131, in <module>
    rv = omero.cli.argv()
  File "/opt/omero/web/venv3/lib64/python3.6/site-packages/omero/cli.py", line 1756, in argv
    cli.invoke(args[1:])
  File "/opt/omero/web/venv3/lib64/python3.6/site-packages/omero/cli.py", line 1187, in invoke
    stop = self.onecmd(line, previous_args)
  File "/opt/omero/web/venv3/lib64/python3.6/site-packages/omero/cli.py", line 1264, in onecmd
    self.execute(line, previous_args)
  File "/opt/omero/web/venv3/lib64/python3.6/site-packages/omero/cli.py", line 1346, in execute
    args.func(args)
  File "/opt/omero/web/venv3/lib64/python3.6/site-packages/omero/plugins/script.py", line 651, in upload
    id = scriptSvc.uploadOfficialScript(args.file, p.text())
  File "/opt/omero/web/venv3/lib64/python3.6/site-packages/path.py", line 814, in text
    return f.read()
  File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 47060: ordinal not in range(128)

but I don't have a problem uploading this script to other servers. e.g. py3-ci etc.

will-moore commented 4 years ago

This is likely due to the env settings in the server docker image, similar to issues we're seeing at https://github.com/ome/omero-figure/pull/351#issuecomment-565392138. With fixes to Docker images, we can revert changes to app-srv in this PR.

snoopycrimecop commented 4 years ago

Conflicting PR. Removed from build OMERO-plugins-push#120. See the console output for more details. Possible conflicts:

Upstream changes
- .omeroci/app-srv

~~--conflicts~~ Conflict resolved in build OMERO-plugins-push#121. See the console output for more details.

will-moore commented 4 years ago

Travis is failing tests with scriptService.getParams() as

    def getParams(self, scriptID, _ctx=None):
>       return _M_omero.api.IScript._op_getParams.invoke(self, ((scriptID, ), _ctx))
E       omero.InternalException: exception ::omero::InternalException
E       {
E           serverStackTrace = 
E           serverExceptionClass = 
E           message = Sha1s don't match! expected 86e60a3df1b2a4a10e144163611710d225eff0f3, found af91ad4687994c8d58a58c2bc5960129291b95b5
E       }

This is probably due to the script containing Unicode characters, causing the script upload to fail. See above: https://github.com/ome/omero-figure/pull/343#issuecomment-565011663 and a fix for Unicode handling in scripts at: https://github.com/ome/omero-py/pull/150

Either that fix hasn't propagated into omero_test_infra yet, or some similar fixes are needed elsewhere?

joshmoore commented 4 years ago

If the script is being manually uploaded outside of OMERO, then you'll need to run the equivalent of scripts list to have it detected.

will-moore commented 4 years ago

But why should this PR fail with Sha1 mismatch when no other PRs have? I previously updated app-srv to use omero script replace but that failed because of Unicode in the script. Then when I realised it was a Unicode issue LANG etc then I removed those changes as I thought the issue would be fixed elsewhere?

will-moore commented 4 years ago

So, if I remove the 2 unicode characters from the script then upload works fine (no sha1 mismatch) and travis is happy.

will-moore commented 4 years ago

To create figures without this PR (see testing above) I'll exclude for now...

...removed 9th Feb

will-moore commented 4 years ago

There is still an issue here with Travis failing when the figure export script contains unicode (see last commit above). That is a temp workaround which I'd like to revert. Any ideas @joshmoore @manics Thx

joshmoore commented 4 years ago

The script does get successfully listed. I would expect at that point that the checksums should match:

+ su - omero-server
Created session for root@omero:4064. Idle timeout: 10 min. Current group: system
 id | Official scripts                       
----+----------------------------------------
 62 | /omero/figure_scripts/Figure_To_Pdf.py 
(1 row)

The only two causes I can think of are:

something changes the script
checksum calculation is broken for unicode strings

pwalczysko commented 4 years ago

@will-moore I think I am ready with the prep without this pr on that dataset - my figures are called scalebarx where x goes from 1 to 9 (note that there is also scalerabsx series, which is just a training and missing the non-scalebared images).

pwalczysko commented 4 years ago

Bug: Newly added scalebars invisible in pdf

Figure url: https://merge-ci.openmicroscopy.org/web/figure/file/18844 Figure name : scalebars8

without this PR: create a figure, add scalebars, except threee last images
with this PR, open the figure and edit the scalebars, also adding some new scalebars on the pre-existing images (on 2 out of 3 of those images)
Save the figure
Export the figure as pdf
Observe that although the new scalebars on old images are there in the figure UI, they are missing in the pdf

Note that I had to upload the Figure to pdf script manually today again.

edit: This was caused only by not using the right Figure-to-pdf.py script. All good here.

pwalczysko commented 4 years ago

Now, I am almost sure the problem https://github.com/ome/omero-figure/pull/343#issuecomment-583504935 is caused by me not using the Figure-to-pdf script from this PR. But I cannot re-upload that script using insight, because the older, release script is already there. Will have to wait till tomorrow, when the script will vanish again I guess...

pwalczysko commented 4 years ago

The retest with this PR passes, see figure https://merge-ci.openmicroscopy.org/web/figure/file/18844/ of user-3. The pdf is also matching.

All good here.

will-moore commented 4 years ago

To test the last commit, check that Angstrom and Micron symbols are correct unicode in Pdf / TIFF export.

jburel commented 4 years ago

The unit is correctly displayed in PDF and tiff I went from angstrom to Micron and ended with out of synch option 0micron on scale bar, and 10 in right-hand panel. I had to enter a value < 1 which then rounded to 0

jburel commented 4 years ago

Last commit looks good @will-moore I don't know if you want to look at this mismatch in this PR

will-moore commented 4 years ago

@jburel I was assuming that user would only use integers for scalebar values. That commit allows floats and seems to be working for me:

Screenshot 2020-02-20 at 16 16 44

pwalczysko commented 4 years ago

Tested the non-integer labels, exported as pdf and tiff. All works fine.

Ready to merge fmpov

jburel commented 4 years ago

./omero_figure/views.py:56:6: N802 function name 'getLengthUnits' should be lowercase

jburel commented 4 years ago

Thanks for making the changes. It works nicely now

ome / omero-figure

Scalebar units #343