Allow TimeSpans to be broadcasted

kendal-s commented 2 years ago

Currently broadcasting TimeSpans returns an error without wrapping the TimeSpan in a Ref. It would be nice to be able to broadcast a Timespan without having to wrap it!

Example:

foo = DataFrame(a=["test"], b=[TimeSpan(0, 100)])

foo.a .= "new test" # works fine! 

new_ts = TimeSpan(0, 200)
foo.b .= new_ts # julia is angry! ❌
foo.b .= Ref(new_ts) # expected behavior! ✅

ararslan commented 2 years ago

You could turn the example you gave in the PR body into a test without the DataFrames dependency by using plain ol' vectors, e.g.

x = [TimeSpan(0, 100)]
x .= TimeSpan(0, 200)
@test x == [TimeSpan(0, 200)]

ararslan commented 2 years ago

Thanks Kendal! If someone is willing, it might be nice to backport this for a v0.2.9 release since (AFAIK) a lot of stuff doesn't yet support TimeSpans v0.3.

kendal-s commented 2 years ago

Thanks for the support on this! I'm not familiar with the process for backporting but I'd love to get insight on how to do it

ararslan commented 2 years ago

Sure thing! Basically it just involves some minor git gymnastics. Starting from an updated clone of the repository,

git checkout release-0.2
git checkout -b kks/backport-pr-43
git cherry-pick -x -e 902c49a210ead85584d5d21373d64f1a1163b88f

The cherry-pick will likely cause a conflict since it changes the Project.toml in a way that doesn't match the state on the release-0.2 branch. The resolution would be to bump the patch version according to the current v0.2 version. Once the change is made and the conflict is resolved,

git add -u
git cherry-pick --continue
git push origin kks/backport-pr-43

then open a PR against the release-0.2 branch. The base branch for a PR is configurable in the GitHub UI. Once that's all squared away, feel free to request me for a review. 🙂

beacon-biosignals / TimeSpans.jl

Allow TimeSpans to be broadcasted #43