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
rework dispatches and update compatibility #32
Conversation
It's much clearer to dispatch on pixel type, rather than the array type. So intead of extend `evaluate`, we extend `eval_op` now.
there're two other patches: * provide limited support to non-boolean image when it can be binarized without InExactError * fix pariwise for GenericHausdorff; the previous function can't get called due to an incorrect type annotation
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 for cleaning the code and improving the integration with other packages in the Julia ecosystem. I didn't have a chance to review things carefully, but I am confident that you are careful with the changes.
Unfortunately, ambiguities still exist (moves from |
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
+ Coverage 69.1% 87.61% +18.5%
==========================================
Files 6 6
Lines 123 113 -10
==========================================
+ Hits 85 99 +14
+ Misses 38 14 -24
Continue to review full report at Codecov.
|
The purpose of this rewrite is to close #22, close #28 and close #25 . Also, it's much easier to understand and maintain the source codes.
This rework consists of three updates:
result_type
on::Type
rather than::AbstractArray
(blocked by rework result_type JuliaStats/Distances.jl#140)eval_op
instead ofevaluate
. The latter introduces a lot of method ambiguities (e.g., avoid type piracy? #28 (comment) reported by @tlnagy)ImageCore >= 0.8.3
--> I'd like to make this a seperate PR but it's required to simplify the dispatching.Todo:
cc: @juliohm