-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Introduce optional prepend
/append
argument for diff
#50945
base: master
Are you sure you want to change the base?
Conversation
julia> diff(a, dims=2, prepend = 0) | ||
2×2 Matrix{Int64}: | ||
0 2 | ||
0 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The syntax is inspired by Numpy.
If we were matching numpy semantics (and not just syntax), it sounds like they prepend before diffing, so with those semantics I'd expect this to look like
julia> diff([[0,0] ;; a]; dims=2)
2×2 Matrix{Int64}:
2 2
6 10
I think with those semantics, one does something like
diff(a, dims=2, prepend = a[:, 1])
to get 0s in the diff like this.
I am not sure which semantics is better but I think it would be good at least if the docstring made sure to specify the way the numpy docstring does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review and comments. I do actually use np.diff
quite often, but my data always start with zero. So, I've never thought if its prepend before or after diff-ing. Here, it would be nice to have similar behaviour to np.diff
since so many people are going back and forth between Julia and Python. I'll update the PR accordingly.
Co-authored-by: Eric Hanson <5846501+ericphanson@users.noreply.github.com>
You could use |
Still not super elegant, but I think the nicest solution might be to have the public facing function diff(a::AbstractArray; dims::Integer, kw...)
prepend = isdefined(kw, :prepend) ? Some(kw.prepend) : nothing
append = isdefined(kw, :append) ? Some(kw.append) : nothing
if !isempty(unsupported = structdiff(kw, NamedTuple{(:prepend, :append)}
throw(ArgumentError(lazy"got unsupported keyword argument(s) $unsupported"))
end
return _diff(a, dims, prepend, append)
end and then handle the rest of the logic via dispatching in |
I would use
prepend
quite often, so I've tried to prepare PR to close #42509.. The syntax is inspired by Numpy.TODOes
nothing
in optional arguments (what is the best here?)Cc: @pdeffebach
Edit:
Using
_nothing
seems like a temporary solution. Any idea on how to improve/fix the implementation?