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

Register this package? #2

Closed
daanhb opened this issue Jul 19, 2022 · 5 comments
Closed

Register this package? #2

daanhb opened this issue Jul 19, 2022 · 5 comments

Comments

@daanhb
Copy link
Member

daanhb commented Jul 19, 2022

@dlfivefifty @MikaelSlevinsky any opinions on whether or not to proceed like this?

@daanhb
Copy link
Member Author

daanhb commented Jul 19, 2022

The code in this package is like that in FastTransforms.jl except that it no longer specializes DSP.conv, in order to reduce the dependencies.
FastTransforms.jl could just use GenericFFT and it would simply still have to define:

import DSP: conv
conv(u::AbstractArray{T, N}, v::AbstractArray{T, N}) where {T<:AbstractFloat, N} =
    GenericFFT._conv!(deepcopy(u), deepcopy(v))
conv(u::AbstractArray{T, N}, v::AbstractArray{Complex{T}, N}) where {T<:AbstractFloat, N} =
    GenericFFT._conv!(complex(deepcopy(u)), deepcopy(v))
conv(u::AbstractArray{Complex{T}, N}, v::AbstractArray{T, N}) where {T<:AbstractFloat, N} =
    GenericFFT._conv!(deepcopy(u), complex(deepcopy(v)))
conv(u::AbstractArray{Complex{T}, N}, v::AbstractArray{Complex{T}, N}) where {T<:AbstractFloat, N} =
    GenericFFT._conv!(deepcopy(u), deepcopy(v))

@dlfivefifty
Copy link
Member

I’ll defer to @MikaelSlevinsky who wrote the original code… but I’m sure it’s a good idea

@MikaelSlevinsky
Copy link
Member

Hi @daanhb, thanks for taking charge of this! I would say that registration is good idea.

As for conv, can't remember the last time I used it, but looking at dspbase.jl, maybe a PR there that changes some of the type restrictions on conv would solve a bunch of issues.

For example, the 3-arg (matrix) conv has no type restriction. Since it uses fft and ifft, if GenericFFT were loaded then I think it would just work beyond FFTWNumbers.

This would mean importing DSP in FastTransforms may also be removable.

@daanhb
Copy link
Member Author

daanhb commented Jul 20, 2022

@JuliaRegistrator register

@JuliaRegistrator
Copy link

Registration pull request created: JuliaRegistries/General/64580

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.1.0 -m "<description of version>" 93a246006dff9c1f0b444183830bd6a8fc72fb2f
git push origin v0.1.0

@daanhb daanhb closed this as completed Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants