-
Notifications
You must be signed in to change notification settings - Fork 0
Usability changes #16
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
Conversation
Rebuild implies to me it already exists, which it may not
If it doesn't already exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @hinerm for making the Python mode better!
@@ -70,7 +70,7 @@ public class OptionsPython extends OptionsPlugin { | |||
@Parameter(label = "Python environment directory", persist = false) | |||
private File pythonDir; | |||
|
|||
@Parameter(label = "Rebuild Python environment", callback = "rebuildEnv") | |||
@Parameter(label = "Build Python environment", callback = "rebuildEnv") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
// Notify user of success | ||
if (uiService != null) { | ||
uiService.showDialog( | ||
"Python environment setup was successful and is ready to use!", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -187,10 +187,13 @@ public void save() { | |||
log.debug(exc); | |||
} | |||
|
|||
if (pythonMode && (pythonDir == null || !pythonDir.exists())) { | |||
rebuildEnv(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
@@ -145,18 +145,25 @@ public void load() { | |||
} | |||
|
|||
public void rebuildEnv() { | |||
// Use scijava.app.python-env-file system property if present. | |||
File environmentYaml = getEnvironmentYamlFile(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should fail fast if the file does not exist?
// HACK: stderr stream triggers console window show. | ||
System.err.println("Building Python environment"); | ||
System.err.println(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
print(type(ij_polygon_roi)) | ||
|
||
# convert index images to ImageJ ROI in RoiManager | ||
#TODO any way to color the selections? We can use Colors... but it appears to be global and the last one run wins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I got different-colored lines working in the Microscope Image Focus Quality plugin. It makes a bunch of line ROIs and adds them to the overlay in order to fake a gradient bar overlay. So there must be a way!
|
||
#TODO this pops an unnecessary display at the end but if I don't make it the last line the ROIs don't show | ||
rm.moveRoisToOverlay(imp) | ||
rm.runCommand(imp, "Show All") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script is totally badass! 💪
if (!trimmed.isEmpty()) yml.append(" - ").append(trimmed).append( | ||
"\n"); | ||
} | ||
yml.append(" - ").append(pyimagej).append("\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should append pyimagej>=1.7.0
only if they did not include it themselves in their list. Otherwise, it might not be possible to change the version range used, and in particular might not be possible to pin to a specific version.
String[] channels = { "conda-forge" }; | ||
String pyimagej = "pyimagej>=1.7.0"; | ||
String apposePython = | ||
"git+https://github.com/apposed/appose-python.git@efe6dadb2242ca45820fcbb7aeea2096f99f9cb2"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not hardcode versions in Java. Can we instead parse them out of the existing shipped environment.yml
when populating the GUI, so they will propagate naturally?
} | ||
yml.append(" - ").append(pyimagej).append("\n"); | ||
yml.append(" - ").append(apposePython).append("\n"); | ||
java.nio.file.Files.write(envFile.toPath(), yml.toString().getBytes()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to tweak how this works, because the Updater currently ships the environment.yml
and since this GUI now allows editing it, that results in a locally modified file. How about if we ship an environment.yml.default
instead, and then if no environment.yml
exists, we fall back to the default
file for populating the package lists here?
I am pretty happy with how the python configuration is working in Fiji with these changes (e.g. notifications for building, auto-open the python settings when failing to run scripts, being able to modify libraries in the environment - though this logic is probably fragile)
But I don't understand where `scyjava.enable_scijava_scripting` is actually called. I expected to have an initialized PyImageJ gateway to use in my scripts, tied to the Fiji installation I started in python mode. But I couldn't figure out how to do so or find where to troubleshoot.Found it.