-
Notifications
You must be signed in to change notification settings - Fork 107
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 UI support for custom Python operations #640
base: master
Are you sure you want to change the base?
Add UI support for custom Python operations #640
Conversation
c8dfc57
to
e142e78
Compare
Current coverage is 53.19% (diff: 18.21%)@@ master #640 diff @@
==========================================
Files 209 213 +4
Lines 6715 6961 +246
Methods 0 0
Messages 0 0
Branches 656 693 +37
==========================================
+ Hits 3665 3703 +38
- Misses 2882 3089 +207
- Partials 168 169 +1
|
I think the "Create New" button needs a margin. When searching for operations, what do you think about getting rid of the categories and just showing operations? |
I think it's good to keep search results separate, it makes it easier to know what category the operations belong to. Unless if it turns out that people use the search bar for the majority of the time. |
Doesn't crash when new python scripts have errors (just alerts the user). Loading erroneous scripts from disk will be logged but not added to the operations list
loader.load(); | ||
return loader.getController(); | ||
} catch (IOException e) { | ||
throw new RuntimeException("Couldn't load python editor", e); |
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.
Ugly
} | ||
|
||
@FXML | ||
private void openWiki() { |
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.
Take a look at how we did it for the AboutDialog
closes #470 |
+ " outputs.createNumberSocketHint(\"sum\", 0.0),\n" | ||
+ "]\n\n\n" // two blank lines | ||
+ "def perform(a, b):\n" | ||
+ " return a + b\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.
Maybe make this a file? What do you think?
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.
That makes some sense. I'd like to keep the contents in memory though to minimize disk accesses.
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.
Put it in a read once file.
Cache the string in memory after its loaded.
It will make it easier to edit in the future.
I think we're going to need to use the official OpenCV python libraries instead of accessing the Java bindings through python. |
Also some minor fixup from PR
Are we waiting on this for the opencv library change? |
Haven't had the time to figure out how to deal with the OpenCV python libraries |
Conflicts: build.gradle core/src/test/java/edu/wpi/grip/core/PythonTest.java core/src/test/java/edu/wpi/grip/core/serialization/ProjectTest.java ui/src/test/java/edu/wpi/grip/ui/PaletteTest.java
So this is going to be interesting. We don't need to include python bindings for OpenCV (you can just use |
Write a library to emulate numpy in jython |
I like this feature: was thinking how to dynamically add OpenCV2 calls into the tool, amd import/export the custom library for sharing |
Wow, this is an old PR. IIRC it never moved forward because it's really just a python wrapper around the java libraries (which are wrappers around the C++ library...), and wouldn't support the normal python libraries people would use to interface with OpenCV like numpy or opencv2 |
Screenshot
Issues
TextArea
, might want to make it more full-featuredCreate New
button