Use IrrationalConstants
and sinpi
/cospi
#40
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR simplifies the package and makes it more consistent with base Julia and the ecosystem. It
tau
(and its unicode alias) as alias ofIrrationalConstants.twoπ
instead of a new irrational constant,modtau
as alias ofmod2pi
instead of a new function,sintau
andcostau
assintau(x) = sinpi(2 * x)
etc. to make use of the implementations in https://github.com/JuliaLang/julia/blob/master/base/special/trig.jl without duplicating them and having to synchronize them,sintau(x::Integer) = zero(float(x))
andcostau(x::Integer) = one(float(x))
instead ofzero(x)
andone(x)
to be consistent with base Julia (https://github.com/JuliaLang/julia/blob/fc9c280584f63236a1b97da4178a41eba65b6da2/base/special/trig.jl#L935-L936 and https://github.com/JuliaLang/julia/blob/fc9c280584f63236a1b97da4178a41eba65b6da2/test/math.jl#L597),By using aliases one gets all existing functionality for
twoπ
andmod2pi
for free.Intentionally I did not fix JuliaMath/IrrationalConstants.jl#20 and #39 to avoid introducing any new functionality and definitions.
The main user-facing change is the re-definition of
sintau(x::Integer)
andcostau(x::Integer)
and thattau isa IrrationalConstant{:twoπ}
. I'd argue that the former is a bugfix but maybe it is safer to make a breaking release, also due to the type change. Probably it does not matter though since the only direct dependent of Tau.jl 0.2.0 was PredictMDExtra (https://juliahub.com/ui/Packages/Tau/jo2zN/0.2.0?page=2) which however does not depend on Tau anymore (https://github.com/bcbi/PredictMDExtra.jl/blob/master/Project.toml, changed in 2019: bcbi/PredictMDExtra.jl#149).cc: @hyrodium