JuliaMath / InverseFunctions.jl

Interface for function inversion in Julia
Other
29 stars 8 forks source link

add divrem and fldmod #24

Closed aplavin closed 2 years ago

codecov[bot] commented 2 years ago

Codecov Report

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

Coverage data is based on head (5fbf09d) compared to base (34c1adf). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #24 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 3 3 Lines 74 87 +13 ========================================= + Hits 74 87 +13 ``` | [Impacted Files](https://codecov.io/gh/JuliaMath/InverseFunctions.jl/pull/24?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath) | Coverage Δ | | |---|---|---| | [src/functions.jl](https://codecov.io/gh/JuliaMath/InverseFunctions.jl/pull/24/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath#diff-c3JjL2Z1bmN0aW9ucy5qbA==) | `100.00% <100.00%> (ø)` | | | [src/inverse.jl](https://codecov.io/gh/JuliaMath/InverseFunctions.jl/pull/24/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath#diff-c3JjL2ludmVyc2Uuamw=) | `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=JuliaMath). 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=JuliaMath)

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

oschulz commented 2 years ago

@aplavin is this more "for completeness", or do you have specific use cases in mind? I'm not oppsed, just curious since Base.Fix2(divrem, x) would seem to be a rather special construction.

aplavin commented 2 years ago

The original motivation is to have div/mod setters in Accessors.jl. First I thought of just adding the corresponding math there, but InverseFunctions does look like the "proper" place for that. So, the plan is to have these inverses here, and then Accessors would just do like set(x, div, y) = set(x, divrem[1], y) (not actual code). Setting divrem would just work because of the inverse.

oschulz commented 2 years ago

Makes sense - @devmotion what do you think?

devmotion commented 2 years ago

I'm not sure. IIRC we don't define inverses for functions that return tuples so far? Do we want to support such functions? Should we start with more common functions such as eg. sincos first?

oschulz commented 2 years ago

inverses for functions that return tuples

Well, as long as the inverse function will take such a tuple as it's input it's fine I would say.

aplavin commented 2 years ago

Should we start with more common functions such as eg. sincos first?

Could also be useful, but it's not invertible...

Agree with code comments, will implement them. Thanks for the detailed review!

aplavin commented 2 years ago

bump

oschulz commented 2 years ago

bump

@devmotion

oschulz commented 2 years ago

@aplavin and @devmotion , is this close to merging? If so, I would hold off on a new release for this PR.

oschulz commented 2 years ago

@aplavin good to merge from your side?