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

Opify static methods #10

Merged
merged 8 commits into from
Aug 29, 2024
Merged

Opify static methods #10

merged 8 commits into from
Aug 29, 2024

Conversation

gselzer
Copy link
Contributor

@gselzer gselzer commented Jun 10, 2024

This WIP PR intends to "opify" the static functionality within the project, and is divided into two subtasks:

  1. Declare all op-like algorithms as Ops using the SciJava Ops Indexer
  2. Migrate all Ops operating on Meshes from scijava-ops-image to here.

I've additionally filed issues for, or just outright added, functionality that was declared to work in scijava-ops-image/ImageJ Ops but was not functional:

Points of discussion:

  • I find the geom.mainElongation and geom.medianElongation Ops to have confusing names, as I could find no literature that named these properties. My understanding is that geom.mainElongation measures the elongation of a cross-section ellipse, where the section is perpendicular to the two larger semi-axes, while geom.medianElongation does the same but with the two smaller semi-axes. @tibuch is this correct? At a minimum I want to document this, but better would be to come up with better names so we can alias the Op.

@gselzer gselzer added the enhancement New feature or request label Jun 10, 2024
And improve the test to make varying the sphere radius simpler.

Migrated from imagej/imagej-ops#653.

See also imagej/imagej-ops#422.

Co-authored-by: Kyle Harrington <[email protected]>
@tibuch
Copy link
Contributor

tibuch commented Jun 18, 2024

Absolutely agree with the confusing naming. Maybe Major- and MinorElongation would be better. Or simply return an ElongationVector similar to EigenValues.

I would definitely drop MedianElongation because there is no median involved.

@hinerm
Copy link
Member

hinerm commented Jul 2, 2024

@tinevez In this PR we are discussing the minimum Java version to index libraries with Ops. I'm wondering if there is a need to keep imglib2-mesh at Java 8, or if going to Java 11 is an option?

@gselzer gselzer marked this pull request as ready for review August 26, 2024 15:30
@tinevez
Copy link
Collaborator

tinevez commented Aug 29, 2024

Hello @hinerm
The next release must be able to be shipped and used within Fiji without modifications.
When Fiji is shipped with Java > 8, then we can do the same here.

@gselzer
Copy link
Contributor Author

gselzer commented Aug 29, 2024

Hi @tinevez, I've written this PR to require Java 11+ compilers (to enable Ops annotation processing), but Java 8 code. It should work just fine in a Java 8 Fiji! Please let me know if you have any questions or concerns with this.

@tinevez
Copy link
Collaborator

tinevez commented Aug 29, 2024

Thanks for the clarification! The difference between compile time requirements and execution time requirements escaped me.
It's all good on my side then.

@tinevez tinevez merged commit 287bd54 into main Aug 29, 2024
@tinevez tinevez deleted the add-ops branch August 29, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants