Wrap custom `jinc` around `SpecialFunctions.jinc`

[roflmaostc]

Hi,

I just discovered the custom jinc method. This would is a breaking change since your exported jinc had a different scaling convention (\pi / 4).

So probably not worth it but in the long run it might be clever to stick to the convention chosen in SpecialFunctions.jl.

Best,

Felix :)

[JeffFessler]

Thanks for the suggestion. I have a pretty strong preference for the conventions in the version of jinc in this package (just as I have a strong preference for sinc with a pi in it). I follow the conventions in Bracewell's classic books on FTs, where jinc is the 2D FT (or Hankel transform) of a disk of unit diameter, so jinc(0) = \pi/4. Now I realize you have put some effort into optimizing the accuracy of jinc in that other PR, so I would be fine with defining the jinc in this package in terms of the one over there, but I still want to export here the one with Bracewell's conventions for scale factors. Having the name conflict for people using both packages is probably a good thing since it will force them to decide which convention they want for their work and qualify using accordingly! I wish I had could have lobbied for Bracewell's convention when the jinc in SpecialFunctions was added. The sinc in Julia has the logical property sinc(0) = 1 because it is the area of a rect of unit width. It would have been logical then for jinc(0)=\pi/4 as the area of a disk of unit diameter. But that ship has sailed it seems...

[roflmaostc]

Ah, I see. Your reasoning makes sense.

Should I revert the commits and simply wrap your jinc in the one of SpecialFunctions?

[JeffFessler]

That would be great. If you undelete jinc.jl then I could take it from there later (in a couple weeks) using your PR as the starting point, but if you are willing to do that wrap yourself then it will get merged sooner! BTW, did you hit the name conflict? Probably we should update the docstring here to explicitly alert the user that the convention differs...

[roflmaostc]

I incorporated your suggestion and removed the promote_type because it should be handled by SpecialFunctions.jinc.

[codecov[bot]]

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage :thumbsup:

Coverage data is based on head (2f1cc85) compared to base (0b307cb). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #77 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 20 20 Lines 594 591 -3 ========================================= - Hits 594 591 -3 ``` | [Impacted Files](https://codecov.io/gh/JuliaImageRecon/ImagePhantoms.jl/pull/77?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/jinc.jl](https://codecov.io/gh/JuliaImageRecon/ImagePhantoms.jl/pull/77?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ppbmMuamw=) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[JeffFessler]

Yay - the tests all pass! One question about compat: what version of SpecialFunctions first got jinc? We might need to update Project.toml?

[roflmaostc]

I think v1.1 so we should be safe with 1.8

https://github.com/JuliaMath/SpecialFunctions.jl/releases/tag/v1.1.0

[JeffFessler]

Thanks! Maybe no need to bump the version right now since this is (hopefully) not really breaking or fixing. And I want to see how the docs look because jinc is used in the spectra there. But just ping me if you think we need to bump the version sooner.

[roflmaostc]

Thanks a lot too! So it was actually just an improvement in terms of accuracy for special cases :smile: