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

fixes comparisons for DateTimeConversion #4803

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/dim-converts/dates-integration.jl
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ For DateTimes `PlotUtils.optimize_datetime_ticks` is used for getting the conver
```julia
date_time = DateTime("2021-10-27T11:11:55.914")
date_time_range = range(date_time, step=Week(5), length=10)
# Automatically chose xticks as DateTeimeTicks:
# Automatically chose xticks as DateTimeTicks:
scatter(date_time_range, 1:10)

# explicitly chose DateTimeConversion and use it to plot unitful values into it and display in the `Time` format:
Expand All @@ -59,7 +59,6 @@ needs_tick_update_observable(conversion::DateTimeConversion) = conversion.type
create_dim_conversion(::Type{<:Dates.AbstractTime}) = DateTimeConversion()
MakieCore.should_dim_convert(::Type{<:Dates.AbstractTime}) = true


function convert_dim_value(conversion::DateTimeConversion, value::Dates.TimeType)
return date_to_number(conversion.type[], value)
end
Expand Down Expand Up @@ -87,7 +86,6 @@ function convert_dim_observable(conversion::DateTimeConversion, values::Observab
end

function get_ticks(conversion::DateTimeConversion, ticks, scale, formatter, vmin, vmax)

if scale != identity
error("$(scale) scale not supported for DateTimeConversion")
end
Expand All @@ -113,3 +111,5 @@ function get_ticks(conversion::DateTimeConversion, ticks, scale, formatter, vmin
return tickvalues, string.(dates)
end
end

Base.:(==)(a::DateTimeConversion, b::DateTimeConversion) = a.type[] == b.type[]
25 changes: 15 additions & 10 deletions src/dim-converts/dim-converts.jl
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,21 @@ function Base.setindex!(conversions::DimConversions, value::Observable, i::Int)
end

function Base.setindex!(conversions::DimConversions, value, i::Int)
isnothing(value) && return # ignore no conversions
conversions[i] === value && return # ignore same conversion
isnothing(value) && return nothing # ignore no conversions
conversions[i] == value && return nothing # ignore same conversion
if isnothing(conversions[i])
# only set new conversion if there is none yet
conversions.conversions[i][] = value
return
return nothing
else
throw(ArgumentError("Cannot change dim conversion for dimension $i, since it already is set to a conversion: $(conversions[i])."))
throw(
ArgumentError(
"Cannot change dim conversion for dimension $i to $value, since it already is set to a conversion: $(conversions[i]).",
),
)
end
end


## Interface to be overloaded for any AbstractDimConversion type
function convert_dim_value(conversions::DimConversions, dim::Int, value)
if isnothing(conversions[dim])
Expand All @@ -45,14 +48,15 @@ function convert_dim_value(conversions::DimConversions, dim::Int, value)
return convert_dim_value(conversions[dim], value)
end


function convert_dim_value(axislike::AbstractAxis, dim::Int, value)
return convert_dim_value(get_conversions(axislike), dim, value)
end

convert_dim_value(::NoDimConversion, value) = value
function convert_dim_value(conversion::AbstractDimConversion, value, deregister)
error("AbstractDimConversion $(typeof(conversion)) not supported for value of type $(typeof(value))")
return error(
"AbstractDimConversion $(typeof(conversion)) not supported for value of type $(typeof(value))"
)
end

using MakieCore: should_dim_convert
Expand All @@ -73,7 +77,6 @@ end
# The below is defined in MakieCore, to be accessible by `@recipe`
# MakieCore.should_dim_convert(eltype) = false


# Recursively gets the dim convert from the plot
# This needs to be recursive to allow recipes to use dim convert
# TODO, should a recipe always set the dim convert to it's parent?
Expand All @@ -99,7 +102,7 @@ function get_conversions(plot::Plot)
end

# For e.g. Axis attributes
function get_conversions(attr::Union{Attributes, Dict, NamedTuple})
function get_conversions(attr::Union{Attributes,Dict,NamedTuple})
conversions = DimConversions()
for i in 1:3
dim_sym = Symbol("dim$(i)_conversion")
Expand Down Expand Up @@ -174,7 +177,9 @@ function needs_tick_update_observable(conversion::Observable)
end
end

function try_dim_convert(P::Type{<:Plot}, PTrait::ConversionTrait, user_attributes, args_obs::Tuple, deregister)
function try_dim_convert(
P::Type{<:Plot}, PTrait::ConversionTrait, user_attributes, args_obs::Tuple, deregister
)
# Only 2 and 3d conversions are supported, and only
if !(length(args_obs) in (2, 3))
return args_obs
Expand Down
Loading