-
Notifications
You must be signed in to change notification settings - Fork 15
feat: add scyjava-stubgen cli command, and scyjava.types namespace, which provide type-safe imports with lazy init
#82
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
base: main
Are you sure you want to change the base?
Conversation
- Introduced `scyjava-stubs` executable for generating Python type stubs from Java classes. - Implemented dynamic import logic in `_dynamic_import.py`. - Added stub generation logic in `_genstubs.py`. - Updated `pyproject.toml` to include new dependencies and scripts. - Created `__init__.py` for the `_stubs` package to expose key functionalities.
…mprove stub generation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
===========================================
+ Coverage 52.72% 75.98% +23.25%
===========================================
Files 12 20 +8
Lines 1303 1653 +350
===========================================
+ Hits 687 1256 +569
+ Misses 616 397 -219 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
hey @ctrueden, I'm struggling to run the jep tests locally (on my mac). I have it's a bit unclear to me whether I should be trying to get these to work at all with jep, or just add a skip to the test |
|
As a workaround, you could try setting |
|
got that working... and commented out the "don't run on macos cause it's flaky" bit, and now get this: that's probably not the flaky bit right? looks like a poor setup of the python environment |
|
i think i see the issue, i'll work on the |
|
after digging a bit deeper into how jgo and jep itself is working, I'm skipping stubgen tests on jep for now. it seems extremely dependent on some careful manual setup of the python environment. I was able to get it to find my standard library, but then it lost the pure python parts of jep. After fixing that, it was unable to find my (editable) install of scyjava because it's not following |
|
code-wise, this is ready to go... however, I don't expect it to be "human-interpretable" yet. There are many ways we could choose to document this and encourage/discourage its usage in various scenarios. Might be best to have a zoom about it so we can tinker with it together and discuss |
|
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/fiji-friends-weekly-dev-update-thread/103718/94 |
|
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/poll-which-priorities-are-most-urgent-for-fijis-python-support/113172/1 |
|
Just a quick update: I removed the testing of jep mode awhile back, so that should no longer gum up the works here. @tlambert03 I'm hoping we can make a little time to revisit this PR some time next week. |
|
yes that would be great! i do think a little chat would be super useful here |
|
Writing some notes to self here (after having not looked at it for a while) that might help others understand what's going on here. This PR...
open questions:to me, the biggest open question here is "who is responsible for generating stubs, and where do they go?". possibilities include:
|
|
after talking with @ctrueden ... something I will try to implement:
for this PR I will pull out everything by the stub generating mechanism, and do the above in another PR |
|
ok @ctrueden, I think this is ready for final review. I've removed the hatch/setuptools plugins, and double checked all the documentation. will follow up on usage patterns in another PR. Let me know if you have any questions! |
|
@ctrueden, any thoughts on this step? I think it stands alone as useful, without imposing/deciding too much on who is going to use it |
|
@tlambert03 Yes, sorry! I started working on a revamp of jgo a few days ago, and it's been eating my entire brain. I will try to review and merge this PR this week! |
|
Just a quick followup to say that I'm looking at this today. I am also considering whether this feature would be best in scyjava or jgo. It will definitely land somewhere, hopefully today. |
|
Yeah I think it could still easily live anywhere... the only thing that might eventually pin it somewhere is a decision to establish a shared public namespace like "scyjava.types". Which is still a big question mark anyway (ie, it make stay forever as a local dev tool) |
Stubs are placed in the scyjava.types namespace by default,
but have imports with a toplevel namespace; for example,
when generating stubs for org.scijava:scijava-common, the file:
src/scijava/types/org/scijava/__init__.pyi
would declare imports:
import java.lang
import java.util
import org.scijava.annotations
import org.scijava.app
import org.scijava.cache
import org.scijava.command
and so on. But these imports yield errors when browsing in an IDE,
and prevent the type checker from resolving all the types properly.
This commit makes the following changes to address the problem:
- Add a python_package_prefix parameter to generate_stubs() function
- Add a _rewrite_stub_imports() function that:
- Rewrites import foo.bar.X → import {python_package_prefix}.foo.bar.X
- Rewrites type references similarly to have the python_package_prefix
- Update the CLI to automatically pass
python_package_prefix="scyjava.types" when using the default location
- If --output-python-path gives a different prefix, use that instead
- Add a new test_stubgen_type_references to validate rewriting behavior
So then the following example above gets rewritten to be:
import java.lang
import java.util
import scyjava.stubs.org.scijava.annotations
import scyjava.stubs.org.scijava.app
import scyjava.stubs.org.scijava.cache
import scyjava.stubs.org.scijava.command
Note that the java.lang and java.util imports are not rewritten,
because they are not among the classes whose stubs are being generated.
With this change, IDEs now autocomplete expressions involving these Java
classes correctly, even when chained; for example:
import scyjava.stubs.org.scijava.Context
Context().getServiceIndex().getA
shows getAll as a valid completion, because getAll() is a member method
of a supertype of ServiceIndex, the class returned by getServiceIndex().
Before this patch, such chained type completions did not work.
Co-authored-by: Claude <noreply@anthropic.com>
|
OK, after reflecting on it, this work is much more suitable here in scyjava than it would be in jgo. The jgo project does not depend on jpype, whereas scyjava is, in a nutshell, unioning the benefits of jgo and jpype. (And now with the stub generation: +stubgenj as well) The only other places I could imagine this work to go would be one of jpype or stubgenj—but we can always explore pushing it upstream later after we run it through its paces. So, in testing this work, I ran into problems making the type completions work in VSCode and PyCharm. The issue was a mismatch between the prefixed classes e.g. Or to put it another way: the stub file at e.g. from scyjava.types.org.scijava import Context
from scyjava.types.org.scijava.ui import UIService
ctx = Context()
ui = ctx.service(UIService).show<ctrl+space>and see the UIService's With 4f54611, this now works, because all the type references in the stubs now get rewritten to have the proper Python package prefix preceding the Java code's own package prefix. @tlambert03 I'm not actually sure though: is the above code how you intended this feature to be used? Or did you want to write: without the Secondly: there remains an issue with core Java classes: this commit does not rewrite imports like Thirdly: due to my lack of knowledge in this area, I leaned rather heavily on Claude to write this patch, and it may have errors or stupidities. What do you think? Are we on the right track here? |
many more details and tests to follow... but just wanted to open this as a WIP.
edit: see #82 (comment) for details
Basic idea, after checking out this branch and running
pip install -e .again:scyjava-stubgen org.scijava:parsington:3.1.0python -c "from scyjava.types.org.scijava.parsington import Function; print(Function(1))". Only at the moment of class instantiation will the jvm be started.