search

JuliaGeometry / GeometryTypes.jl

Geometry types for Julia
Other
67 stars 41 forks source link

Add width getter function for HyperCube #195

Closed mforets closed 4 years ago

SimonDanisch commented 4 years ago

That seems to be exactly the same definition as widths?

SimonDanisch commented 4 years ago

Ah, sorry.. I forgot that HyperCube does actually store it's width as a single float... Hm, i'm still not sure if I like that definition :D Seems a bit ambigious - maybe this doesn't need to be a function, and if one has a hypercube, one should just access the field? We usually only need to have this defined as a function, if we plan to follow an interface that spans multiple types... But I don't really see how the definition of width can easily span over other multi dimensional geometry types ;)

mforets commented 4 years ago

one should just access the field?

If you decide to change the internal representation, or field names of the HyperCube in the future, downstream code accessing the field directly will break. If a getter function is provided, this is less likely. That was the motivation for this PR :) I can do widths(X)[1] but that creates an unnecessary 3d vector.

sjkelly commented 4 years ago

I think this is okay since widths gives a vector and width a scalar. You could feasibly define width for other equally-sized geometries such as HyperSpheres and n-gons.

SimonDanisch commented 4 years ago

Well, I guess we can add this ;)

SimonDanisch commented 4 years ago

Maybe not export it, since it seems like a widely used function^^

mforets commented 4 years ago

Bump?

You could feasibly define width for other equally-sized geometries such as HyperSpheres

fwiw, HyperSpheres have a radius getter function.

Maybe not export it,

I didn't add any export statement in this branch, but width is already exported (https://github.com/JuliaGeometry/GeometryTypes.jl/blob/master/src/GeometryTypes.jl#L186).

SimonDanisch commented 4 years ago

Oh well^^ Lets merge it then ;)