Hi. Le lun. 13 janv. 2020 à 04:39, Matt Juntunen <matt.juntu...@hotmail.com> a écrit : > > Gilles, > > > How about keeping "Transform" for the interface name and define a method > > ... boolean isAffine(); > > I would prefer to have separate types for each kind of transform. > This would make the API clear and would avoid numerous checks in the code in > order to see if a particular transform instance is supported. The transform > types also generally have an "is-a" relationship with each other, which seems > like a perfect fit for inheritance. [1] > > > I don't get that it is an "accuracy" issue. If some requirement is not met, > results will be plain wrong > > Yes, you are correct. I was not very clear in what I wrote. The results will > be completely unusable if the transform does not meet the requirements. > > > I wonder why the documented requirement that an "inverse transform > must exist" does not translate into a method ... getInverse(); > > Good point. All current implementations are able to provide an inverse so > that method should be present on the interface. > > In regard to renaming the Transform interface, I had another idea. The main > purpose of that interface is to provide a way for the partitioning code in > the core module to implement generic transforms of BSP trees (see > AbstractBSPTree.transform()). This is what leads to the requirement that the > transform preserve parallelism, since otherwise, the represented region may > be warped in such a way as to make the tree invalid. However, as far as I can > tell, there is not a general mathematical term for this type of transform > that applies to Euclidean and non-Euclidean geometries. So, my thought is to > move the Transform interface to the "partitioning" package to indicate its > relationship to those classes and simply name it something descriptive like > "ParallelismPreservingTransform" ("parallelism" since that is the more > generic, non-Euclidean form of the concept of "parallel"). The Euclidean > module could then provide a true "AffineTransform" interface that extends > "ParallelismPreservingTransform". The spherical module transforms would > directly inherit from "ParallelismPreservingTransform" and thus avoid any > incorrect usage of the term "affine". The class hierarchy would then look > like this: > > commons-geometry-core > ParallelismPreservingTransform > > commons-geometry-euclidean > AffineTransform extends ParallelismPreservingTransform > AffineTransformXD extends AffineTransform > AffineTransformMatrixXD implements AffineTransformXD > Rotation3D extends AffineTransform3D > QuaternionRotation implements Rotation3D > > commons-geometry-spherical > Transform1S implements ParallelismPreservingTransform<Point1S> > Transform2S implements ParallelismPreservingTransform<Point2S> > > I think the type names here are much more descriptive and mathematically > accurate. WDYT?
I'm not convinced that such a hierarchy would enhance clarity. If the "Transform" is intimately related to the "core" and there is a single set of properties that make it "affine" (and work correctly), I'd tend to keep the name "Transform". [As long as unit tests ensure that concrete implementations possess the expected properties, we are safe.] Regards, Gilles > -Matt > > > [1] https://en.wikipedia.org/wiki/Geometric_transformation > > ________________________________ > From: Gilles Sadowski <gillese...@gmail.com> > Sent: Wednesday, January 8, 2020 8:16 AM > To: Commons Developers List <dev@commons.apache.org> > Subject: Re: [geometry] Rename Transform to AffineTransform > > Hi. > > Le mer. 8 janv. 2020 à 04:39, Matt Juntunen > <matt.juntu...@hotmail.com> a écrit : > > > > Gilles, > > > > > I thought that the question was how to replace "transform"... > > > > I should probably clarify. I want to change the name of the Transform class > > to make it clear that it only represents transforms that preserve > > parallelism (eg, affine transforms). The most obvious name would be > > AffineTransform > > How about keeping "Transform" for the interface name and define a method > ---CUT--- > /** > * Move here the doc explaining under what conditions this method can > return "true". > */ > boolean isAffine(); > ---CUT--- > ? > > Gilles > > > like I suggested but I want to make sure that no one objects to this for > > design or mathematical reasons. > > > > > Anyways, what would be the issue(s) of a non-affine transform? > > > IOW why should implementations of "Transfrom" be restricted to affine > > > transform? > > > > Instances of Transform are used to transform hyperplanes and > > hyperplane-bounded regions. If the transform is not affine, then the > > computed results will not be accurate. > > I don't get that it is an "accuracy" issue. If some requirement is not met, > results will be plain wrong; so it depends on usage: when the transform > must be affine, the code being passed that instance should be able to > check whether it can use it for the intended purpose. > > I wonder why the documented requirement that an "inverse transform > must exist" does not translate into a method > ---CUT--- > Transform<P> getInverse(); > ---CUT--- > > Regards, > Gilles > > > -Matt > > ________________________________ > > From: Gilles Sadowski <gillese...@gmail.com> > > Sent: Tuesday, January 7, 2020 6:41 PM > > To: Commons Developers List <dev@commons.apache.org> > > Subject: Re: [geometry] Rename Transform to AffineTransform > > > > Le mar. 7 janv. 2020 à 17:55, Matt Juntunen > > <matt.juntu...@hotmail.com> a écrit : > > > > > > Gilles, > > > > > > > "AffineMap" (?) > > > > > > I think any name that doesn't include the word "transform" somehow would > > > probably be confusing. > > > > This is supposed to be a synonym.[1] > > I thought that the question was how to replace "transform"... > > > > > > > > > Was the same "Transform" interface used in both the "euclidean" and the > > > "spherical" packages of "Commons Math"? > > > > > > Indirectly. SphericalPolygonsSet extended AbstractRegion, which included > > > an applyTransform(Transform) method. > > > > So the "affine" requirement (in the doc) applied there too. > > > > Anyways, what would be the issue(s) of a non-affine transform? > > IOW why should implementations of "Transfrom" be restricted to affine > > transform? > > > > Regards, > > Gilles > > > > [1] https://en.wikipedia.org/wiki/Affine_transformation > > > > > -Matt > > > ________________________________ > > > From: Gilles Sadowski <gillese...@gmail.com> > > > Sent: Tuesday, January 7, 2020 10:29 AM > > > To: Commons Developers List <dev@commons.apache.org> > > > Subject: Re: [geometry] Rename Transform to AffineTransform > > > > > > Hello. > > > > > > Le mar. 7 janv. 2020 à 16:00, Matt Juntunen > > > <matt.juntu...@hotmail.com> a écrit : > > > > > > > > Hi all, > > > > > > > > The documentation for the o.a.c.geometry.core.Transform interface > > > > (which comes from the original commons-math version) states that > > > > implementations must be affine. Therefore, I think we should rename > > > > this interface to AffineTransform to avoid confusion with other types > > > > of transforms, such as projective transforms. Potential problems with > > > > this are > > > > - the fact that the JDK already has a class with the same name > > > > (java.awt.geom.AffineTransform), and > > > > > > "AffineMap" (?) > > > > > > > - I'm not sure if the term "affine" can technically be applied to > > > > non-Euclidean geometries, such as spherical geometry (there may be > > > > nuances there that I'm not aware of). > > > > > > Was the same "Transform" interface used in both the "euclidean" and the > > > "spherical" packages of "Commons Math"? > > > > > > Regards, > > > Gilles > > > > > > > Anyone have any insight or opinions on this? I've created GEOMETRY-80 > > > > to track this issue. > > > > > > > > Regards, > > > > Matt > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org