feat: add option to autofetch-java with cjdk if available#80
feat: add option to autofetch-java with cjdk if available#80ctrueden merged 8 commits intoscijava:mainfrom
Conversation
f762cfe to
34bfae2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #80 +/- ##
==========================================
- Coverage 52.88% 52.72% -0.17%
==========================================
Files 12 12
Lines 1299 1303 +4
==========================================
Hits 687 687
- Misses 612 616 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
71ef66b to
ece66ce
Compare
* Include java version in job names, to disambiguate them. * As a hack for now, just fail if Java >= v17 or <8.
|
Thanks, @tlambert03 !! Getting the CI to pass was "fun"—even though I'm not sure how I feel about the extra; cjdk is such a small dependency that I'd be inclined to just leave it as a regular ol' dep and dispense with some of the case logic here, knowing it's always available. Would you be OK with that? |
Ah, that's interesting. I can say that I'm confident locally that removing either maven or java does exercise the logic, but yeah: I hadn't yet confirmed for myself on my other PR that removing the setup action actually left the system java-less.
Absolutely, just didn't want to assume. Also, feel free to change the Lastly... I'm curious about your replacement of pipe with Union, even with the |
Oh, no, I was just trying to fix an error regarding the $ cat go.py
def go(a: bool | None) -> None:
print("Hello")
go(True)
$ python go.py
Traceback (most recent call last):
File "go.py", line 1, in <module>
def go(a: bool | None) -> None:
TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'But I have since learned—while troubleshooting setuptools's breaking change to the |
|
... but you can still use the pipe on 3.8 as long as you have ... so in your example, add |
|
i just pushed an update including cjdk by default, and making |
And bump the minimum Python version to 3.9, since that version of setuptools requires it. And raise the test ceiling from Python 3.12 to 3.13.
|
thanks for considering this PR. the prospect of not having to require end users to install java separately definitely makes me happy! :) |
Me too! 😸 Future steps:
In the meantime, for scyjava on conda-forge: I guess we should probably get cjdk onto conda-forge as well, so that the conda dependencies are better aligned... |
|
Yep, I’ll handle cjdk on conda forge |
|
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/87 |
* Remove openjdk and maven dependencies in favor of cjdk instead; see: - scijava/scyjava#80 - scijava/scyjava@5753a59 * Update python and setuptools minimum versions; see: - scijava/scyjava@0af3f65 * Remove jpype1 and jgo from build dependencies. This is a pure Python project, and should not need these runtime dependencies to build the package. * Document why there is a JPype upper bound.
this PR adds logic to automatically setup a competent java environment (with a JRE and maven) if one is not already available and
cjdkis installed (either manually, or via the newscyjava[cjdk]extra). The goal is to enable someone to simplypip install scyjava[cjdk], without having java or maven installed on their system, and have it just work.there is a new
fetch_javaargument tostart_jvm, which:cjdkis importable.cjdkis not available.happy to change those defaults too!
(note: this work came from tlambert03/bffile#3 ... but it seems like it might be reasonable to have here too)