search

beacon-biosignals / TimeSpans.jl

A Julia package that provides a `TimeSpan` type for representing a continuous span between two points in time.
Other
6 stars 2 forks source link

extend `Base.contains` #39

Open palday opened 2 years ago

palday commented 2 years ago

extend Base.contains using a precise type signature so that contains can be used without qualification. No need to export something from Base.

ararslan commented 2 years ago

While I'm definitely sympathetic to this (I've wanted it for quite some time), it was purposefully excluded. See https://github.com/beacon-biosignals/TimeSpans.jl/pull/5#discussion_r573117667.

Note also that as implemented, this constitutes a breaking change; it will no longer Just Work™ for arbitrary arguments that implement start and stop.

ararslan commented 2 years ago

I wonder if there's a different verb we can use so that have our cake and eat too, so to speak, i.e. we can leave the arguments untyped for fans of duck typing and also export without conflicting with a Base function. Could be worth an issue to discuss.

omus commented 2 years ago

If we want to keep using contains as a verb we can always declare our own function (like we were doing) but also fall back on Base for other cases. 

TimeSpans.contains(a::TimeSpan, b::Union{TimeSpan,Period}) = ...
TimeSpans.contains(a, b) = Base.contains(a, b)

This allows users to opt-in to the TimeSpans change but not have to always qualify their usage.