Only code reorganization (src/ directory)#100
Only code reorganization (src/ directory)#100thimotedupuch wants to merge 3 commits intoJuliaDSP:masterfrom
Conversation
|
Thanks for taking time to clean up. One thing I'm not sure is ok is having files with same name modulo case (e.g. |
Okay, then what would be the ideal file structure ? |
|
You could just rename the lowercase files to |
|
Nice, thank you for fixing the tests! |
|
A lot of these are personal preferences, some communities like SciML have style guides, and there are recommendations on Julia's discourse if you search for them |
wheeheee
left a comment
wheeheee
left a comment
There was a problem hiding this comment.
not all comments are related to the reorganisation, but overall LGTM?
| name(::$TYPE{N1,N2}) where {N1,N2} = string($NAMEBASE, N1, "/", N2) | ||
| vanishingmoments(::$TYPE{N1,N2}) where {N1,N2} = (N1, N2) | ||
| end | ||
| for i in length(RANGE1) |
There was a problem hiding this comment.
weird, probably 1:length(RANGE1)? but it somehow works because it's one element long, and 1[1] is ok in julia...
| function nspin2circ(nspin::Tuple, i::Int) | ||
| c1 = CartesianIndices(nspin)[i].I | ||
| c = Vector{Int}(undef, length(c1)) | ||
| for k in 1:length(c1) | ||
| c[k] = c1[k] - 1 | ||
| end | ||
| return c | ||
| end |
There was a problem hiding this comment.
isn't this just collect(c1 .- 1)
| function ($Xwt!)(y::WPTArray{T}, scheme::GLS, L::Integer) where {T<:ValueType} | ||
| return ($Xwt!)(y, scheme, maketree(length(x), L, :full)) | ||
| end |
There was a problem hiding this comment.
x looks like it is undefined in the function?
| nx = nout<<1 | ||
| out::AbstractVector{<:Number}, iout::Integer, nout::Integer, | ||
| x::AbstractVector{<:Number}, ix::Integer, | ||
| shift::Integer=0, ss::Bool=false) where {T<:Number} |
There was a problem hiding this comment.
this may be the autoformatter's work on copy and paste, but I think it's fine without curlies. also, a bit noisy.
There was a problem hiding this comment.
seems like jujutsu is not so good at tracking renamed files? (Transforms.jl also.) git mv would have cut down the diffs
| using ..Util | ||
|
|
There was a problem hiding this comment.
| using ..Util |
using Util while in Util seems redundant, although it doesn't error.
| @@ -0,0 +1,454 @@ | |||
| using ..WT | |||
There was a problem hiding this comment.
| using ..WT |
this too
|
Yeah, thanks for the review |
3 participants
@wheeheee This is the V2 of last PR.
This time I only reorganized the code inside the
src/directory, I did not change any code logic.The code was previously using the
modsubdirectory, that has been deleted and now follows the structure :The reexport dependency is not needed anymore (I believe ?). Also the functions from the different submodules are all exported, as well as the submodules themselves (except for
Util).Here's an overview of the diff :
If this gets merged I will be able to start actually working on the code.
@wheeheee for the logo we'll see later.