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

ENH: Add ConvertPyImageJ example #23

Merged
merged 4 commits into from
May 5, 2022
Merged

Conversation

thewtex
Copy link
Contributor

@thewtex thewtex commented May 2, 2022

No description provided.

@thewtex
Copy link
Contributor Author

thewtex commented May 2, 2022

@ctrueden @joshmoore please take a look. What do you think is the right approach for the attrs regarding zarr serialization

@joshmoore
Copy link

Hey, @thewtex. Nice notebook! For my part, I'm not sure there's a completely right approach yet, but here are some thoughts:

@thewtex
Copy link
Contributor Author

thewtex commented May 3, 2022

Thanks for the tips @joshmoore !

Neat!

I do not know a lot about scifio.metadata.image (@ctrueden is an expert there). I created #28 to track. For now, I will clear the additional metadata so we can successfully create a basic NGFF.

I am pondering if we can use the metadata extension as a stop-gap for itk.Image direction support until further transformation support lands 🤔 .

Nice!

@thewtex thewtex marked this pull request as ready for review May 3, 2022 18:51
@thewtex thewtex force-pushed the pyimagej branch 4 times, most recently from 1c9181e to c0608dd Compare May 3, 2022 21:01
@thewtex
Copy link
Contributor Author

thewtex commented May 3, 2022

Seems to have problems fetching ImageJ?

# initialize ImageJ2
24
ij = imagej.init('2.5.0')
25
print(f"ImageJ2 version: {ij.getVersion()}")
26
---------------------------------------------------------------------------
27
CalledProcessError                        Traceback (most recent call last)
28
File /opt/hostedtoolcache/Python/3.9.12/x64/lib/python3.9/site-packages/jgo/jgo.py:274, in run_and_combine_outputs(command, *args)
29
    273     _logger.debug(f"Executing: {command_string}")
30
--> 274     return subprocess.check_output(command_string,stderr=subprocess.STDOUT)
31
    275 except subprocess.CalledProcessError as e:
32

33
File /opt/hostedtoolcache/Python/3.9.12/x64/lib/python3.9/subprocess.py:424, in check_output(timeout, *popenargs, **kwargs)
34
    422     kwargs['input'] = empty
35
--> 424 return run(*popenargs,stdout=PIPE,timeout=timeout,check=True,
36
    425 **kwargs).stdout
37

38
File /opt/hostedtoolcache/Python/3.9.12/x64/lib/python3.9/subprocess.py:528, in run(input, capture_output, timeout, check, *popenargs, **kwargs)
39
    527     if check and retcode:
40
--> 528         raise CalledProcessError(retcode, process.args,
41
    529                                  output=stdout, stderr=stderr)
42
    530 return CompletedProcess(process.args, retcode, stdout, stderr)
43

44
CalledProcessError: Command '('/usr/bin/mvn', '-B', '-f', '/home/runner/.jgo/net.imagej/imagej/5f34b9963e566d47fe91383d53a5332bfc13df00c5d2f4bd13e2ea10b8f5fb2e/pom.xml', 'dependency:resolve')' returned non-zero exit status 1.
45

46
During handling of the above exception, another exception occurred:
47

48
SystemExit                                Traceback (most recent call last)
49
    [... skipping hidden 1 frame]
50

51
Input In [3], in <cell line: 2>()
52
      1 # initialize ImageJ2
53
----> 2 ij = imagej.init('2.5.0')
54
      3 print(f"ImageJ2 version: {ij.getVersion()}")

@ctrueden
Copy link

ctrueden commented May 4, 2022

CalledProcessError: Command '('/usr/bin/mvn', '-B', '-f', '/home/runner/.jgo/net.imagej/imagej/5f34b9963e566d47fe91383d53a5332bfc13df00c5d2f4bd13e2ea10b8f5fb2e/pom.xml', 'dependency:resolve')' returned non-zero exit status 1.`

Please check this section of the Troubleshooting guide.

First thing to try is:

import imagej.doctor
imagej.doctor.debug_to_stderr(debug_maven=True)

before imagej.init, to get more output on the mvn call.

Another thing you could try (not explained in the Troubleshooting guide yet) is to clear caches. There are two of them: the Maven cache in ~/.m2 and the jgo cache in ~/.jgo. You can rm -rf .jgo without any harm, and it will refresh which version of artifacts are selected when initializing without version locking (e.g. imagej.init() no-args) (and avoid an unfortunate jgo environment caching bug). You can also rm -rf ~/.m2/repository but Maven caches a lot of JAR files there, so this will trigger redownloads of a substantial number of things. You can alternately be more targeted and do only rm -rf ~/.m2/repository/net/imagej/imagej to force it to look again for e.g. net.imagej:imagej:2.5.0. You can alternately feed mvn the -U flag for this, and jgo has a toggle for this, but the scyjava library does not use it unfortunately, so AFAICT from the source code, scyjava does not expose any way to pass -U to mvn (we should fix that).

Now that I have written all that: I guess you probably got this error on a cloud machine somewhere, eh @thewtex? So it was probably starting with a fresh environment and unlikely that local caches were stale, since presumably there weren't any. In that case, I'm not sure what's going on here... but the imagej.doctor.debug_to_stderr(debug_maven=True) will hopefully give us more details to understand better.

"outputs": [],
"source": [
"import sys, os\n",
"!conda install --yes --prefix {sys.prefix} -c conda-forge openjdk=8\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you need to use conda at all, since you use setup-java to install openjdk and maven anyway, which are the non-Python dependencies of pyimagej. You could probably just skip this step, and set JAVA_HOME correctly to wherever setup-java puts it, no?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you need it for Binder, though? It's fine then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good idea @ctrueden ! I tried conda, then moved to what you have set up in CI for pyimagej -- I will try first removing conda then increment on the other debugging steps you suggested.

"Failed to guess the Java version.\n",
"Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=gasp\n",
"WARNING: An illegal reflective access operation has occurred\n",
"WARNING: Illegal reflective access by net.imagej.patcher.LegacyInjector (file:/home/matt/.jgo/net.imagej/imagej/5f34b9963e566d47fe91383d53a5332bfc13df00c5d2f4bd13e2ea10b8f5fb2e/ij1-patcher-1.2.2.jar) to method java.lang.ClassLoader.findLoadedClass(java.lang.String)\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you are using Java 11. Can you tell setup-java to use Java 8 instead? It's not a huge deal, Java 11 should work, but you'll avoid these reflection errors. And Java 17 will unfortunately have problems with the bytecode patching that ImageJ2 does around the original ImageJ.

"Attributes:\n",
" rois: None\n",
" tables: None\n",
" scifio.metadata.image: io.scif.FieldPrinter@1ca3d25b\\n\\t--class io.scif...\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ach, yeah, we can do better here. I filed imagej/pyimagej#200.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

],
"source": [
"# Clean attrs that are Java objects and cannot be serialized to Zarr\n",
"image_da.attrs.clear()\n",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal with imagej/pyimagej#200 would be to eliminate the need to do this. If a Java-side attribute can't be converted to something Pythonic, I think it's OK to drop it. We could at least make it configurable somehow.

@thewtex thewtex changed the title WIP: ENH: Add ConvertPyImageJ example ENH: Add ConvertPyImageJ example May 5, 2022
@thewtex
Copy link
Contributor Author

thewtex commented May 5, 2022

@ctrueden using the setup-java action and pre-installing ImageJ outside the notebook seems to work. Thanks for the tips!

@thewtex thewtex merged commit 1131301 into spatial-image:main May 5, 2022
@thewtex thewtex deleted the pyimagej branch May 5, 2022 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants