rafaqz / Extents.jl

A shared Extent object for Julia spatial data, with DE-9IM spatial predicates
MIT License
4 stars 3 forks source link

Forward `propertynames` to bounds #25

Closed felixcremer closed 4 months ago

felixcremer commented 4 months ago

I am not sure whether this is intentional, because we only want to have access to the dimensions separately but accessing the bounds key of an extent is throwing an error. This is especially confusing because it tab completes only to ext.bounds and not the separate dimensions to then error.

julia> ext = Extent(X=(1,2),Y=(0,4))
Extent(X = (1, 2), Y = (0, 4))

julia> ext.bounds
ERROR: Extent has no field bounds
Stacktrace:
 [1] getproperty(ext::Extent{(:X, :Y), Tuple{Tuple{Int64, Int64}, Tuple{Int64, Int64}}}, key::Symbol)
   @ Extents ~/.julia/packages/Extents/LzSHL/src/Extents.jl:53
 [2] top-level scope
   @ REPL[45]:1
rafaqz commented 4 months ago

Ah maybe propertynames or some other method is missing?

felixcremer commented 4 months ago

The problem is, that the getproperty is basically forwarded to the bounds. So we have to also forward propertynames. We have to decide, whether we would like to also be able to access the bounds as a whole or only separately. I can prepare a PR either way.

This would be the getproperty definition if we would like to also access the bounds all together.

julia> function Base.getproperty(ext::Extent, key::Symbol)
           key == :bounds && return getfield(ext, :bounds)
           haskey(bounds(ext), key) || throw(ErrorException("Extent has no field $key"))
           getproperty(bounds(ext), key)
       end
rafaqz commented 4 months ago

I think bounds will need to use getfield manually.

There is a bounds function too, probably preferable. Not sure I like the inconsistency of getproperty hacks

rafaqz commented 4 months ago

So we just need to forward propertynames