You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Angle objects should be comparable to Quantity/Unit objects. #416
Currently Angle objects can only be compared to other Angle objects. They should be comparable to Quantity objects that make sense (e.g. equivalent to angles).
Ah, I see. I'm a little concerned about the potential for confusion (e.g. thinking q = Quantity(10, u.radian) is the same thing as a = Angle(10,unit=u.radian) and thus trying q.degrees or similar). But maybe I'm over-thinking it?
@astrofrog False. By default, 365 degrees becomes 5 degrees, which is less than 10 degrees. You have to explicitly specify larger bounds (e.g. bounds=[-720,720]) for the internal representation to store angles greater than 360 degrees. The angle above would behave as:
>>> a = Angle(365, unit=u.degree)
>>> print a.degrees
5.0
@demitri - my question was more rethorical - what should it do? While I think it makes sense to compare separations, I don't think it necessarily makes sense to compare absolute angles. This becomes especially fishy if you start considering angles with different bounds for example. For instance, what about comparing -90 degrees (without bounds [-180:180]) with +270 degrees? (with bounds [0:360]). Technically speaking they are the same angle, but the internal values are different. So maybe comparisons should be disabled for absolute angles?
@astrofrog What I listed above is what I believe it should do. I don't think it becomes a problem with different bounds. Remember, to me the Angle object represents the Platonic ideal (as it were), not a decimal value and a unit string. It's an angle. Ignoring bounds, -90 deg is exactly equal to +270. It's the same angle, regardless of how it's specified. This would be ridiculous:
If one want to keep track of the exact values that define the angle (e.g. "-90" and "+270"), let the use do that on their own. They're not dealing with angles anymore, they are dealing with strings and decimals.
There are cases where one wants a different range, but either way, the angle is 40 degrees. The meaning of the angle does not depend on its representation (internal or external). This is very important.
Bounds are also used for error checking in cases where one wants this behavior:
>>> x = 270.0 # e.g. values read from file
>>> a5 = Angle(x * u.deg, bounds=[-90,90]
Exception: out of bounds
An argument can be made that 400 degrees might represent a rotation about an axis of one full revolution plus 40 degrees, and then that would be greater than 40 degrees. But this class is not advertised as representing rotations; it's an angular measure. The above (if needed) should be a subclass of Angle called AngularRotation.
@demitri - I realize I wasn't clear with what I meant - I was talking about disabling specifically the inequality comparisons, which I don't think make sense for a 'platonic' angle - for instance, what does it mean that 350 is larger than 5 degrees, since it's also -10 degrees, which is smaller than 5 degrees. Quantity objects allow inequality comparisons, but I'm just saying that this should be disabled for the Angle sub-class - does this make sense?
Of course, the equality comparison makes sense the way you describe.
@astrofrog I don't think inequality comparisons should be disabled. 350 deg is greater than 5 degrees. -10 degrees is greater than 5 degrees. If you drew this on a piece of paper this is immediately evident. Think of pulling pieces out of a pie chart. The abstract "angle" is a pie piece, whose size is defined by a point of origin, but the angular distance covered is not dependent on a point of origin. Don't be concerned with the actual scalar values - compare the angles defined. I don't see a problem here. This is my concern with Angle being a subclass of Quantity; the latter appears to be defined by the scalar quantities and literal unit, while the former uses that only to determine the angle.
@demitri - I'm confused by what you said here. -10 degrees is not necessarily great than 5 degrees. An analogy might be the vector dot product of point A=(1,2) onto the x-axis and point B=(-2,1) onto the x-axis. I'd say A.x > B.x, because 1 > -2, even though the magnitude B.x is greater than A.x .
Put another way, it's inherently ambiguous whether or not an angle's sign is meaningful or not. As you said above, that could be an AngularRotation, but it's not immediately obvious which one we mean in the Angle class, and there's no way to make it unambiguous because of the imprecision of the word "Angle."
So while I may be wrong, I think@astrofrog is suggesting just disabling it entirely. If we did that, we already have an AngularSeparation, so we could just add an Angle subclass AngularRotation. They could then be nearly-trivial subclasses that treat inequalities different (and possibly also do different things with bounds?).
Activity
eteq commentedon Dec 18, 2012
@demitri - strictly speaking, this means only dimensionless quantities. Is that what you were thinking?
demitri commentedon Dec 19, 2012
It's been too long - I'm not sure what I was thinking. Maybe I was thinking this:
eteq commentedon Dec 19, 2012
Ah, I see. I'm a little concerned about the potential for confusion (e.g. thinking
q = Quantity(10, u.radian)is the same thing asa = Angle(10,unit=u.radian)and thus tryingq.degreesor similar). But maybe I'm over-thinking it?demitri commentedon Dec 19, 2012
Yes. :) If people try that, they will get an error, feel foolish, mend their code and their ways, and lead a fuller, happier life.
eteq commentedon Dec 19, 2012
Ok, that's fine - I'm not sure I'll be able to get to this too soon, so @demitri, do you want to do this PR?
eteq commentedon May 13, 2013
This is being addressed in #1006 , so once that's merged, this can be closed. I'm re-tagging it v0.3 to match #1006
astrofrog commentedon May 13, 2013
We should probably add some tests to ensure that straight comparisons work properly once #1006 is done.
astrofrog commentedon May 13, 2013
By the way, here's a trick question: what should
Angle(365, unit=u.degree) > Angle(10, unit=u.degree)return? :)demitri commentedon May 13, 2013
@astrofrog False. By default, 365 degrees becomes 5 degrees, which is less than 10 degrees. You have to explicitly specify larger bounds (e.g. bounds=[-720,720]) for the internal representation to store angles greater than 360 degrees. The angle above would behave as:
At least that was the API design.
astrofrog commentedon May 13, 2013
@demitri - my question was more rethorical - what should it do? While I think it makes sense to compare separations, I don't think it necessarily makes sense to compare absolute angles. This becomes especially fishy if you start considering angles with different bounds for example. For instance, what about comparing -90 degrees (without bounds [-180:180]) with +270 degrees? (with bounds [0:360]). Technically speaking they are the same angle, but the internal values are different. So maybe comparisons should be disabled for absolute angles?
demitri commentedon May 13, 2013
@astrofrog What I listed above is what I believe it should do. I don't think it becomes a problem with different bounds. Remember, to me the
Angleobject represents the Platonic ideal (as it were), not a decimal value and a unit string. It's an angle. Ignoring bounds, -90 deg is exactly equal to +270. It's the same angle, regardless of how it's specified. This would be ridiculous:If one want to keep track of the exact values that define the angle (e.g. "-90" and "+270"), let the use do that on their own. They're not dealing with angles anymore, they are dealing with strings and decimals.
The point of bounds is to allow this:
instead of the default:
There are cases where one wants a different range, but either way, the angle is 40 degrees. The meaning of the angle does not depend on its representation (internal or external). This is very important.
Bounds are also used for error checking in cases where one wants this behavior:
demitri commentedon May 13, 2013
An argument can be made that 400 degrees might represent a rotation about an axis of one full revolution plus 40 degrees, and then that would be greater than 40 degrees. But this class is not advertised as representing rotations; it's an angular measure. The above (if needed) should be a subclass of Angle called
AngularRotation.astrofrog commentedon May 14, 2013
@demitri - I realize I wasn't clear with what I meant - I was talking about disabling specifically the inequality comparisons, which I don't think make sense for a 'platonic' angle - for instance, what does it mean that 350 is larger than 5 degrees, since it's also -10 degrees, which is smaller than 5 degrees.
Quantityobjects allow inequality comparisons, but I'm just saying that this should be disabled for the Angle sub-class - does this make sense?Of course, the equality comparison makes sense the way you describe.
demitri commentedon May 14, 2013
@astrofrog I don't think inequality comparisons should be disabled. 350 deg is greater than 5 degrees. -10 degrees is greater than 5 degrees. If you drew this on a piece of paper this is immediately evident. Think of pulling pieces out of a pie chart. The abstract "angle" is a pie piece, whose size is defined by a point of origin, but the angular distance covered is not dependent on a point of origin. Don't be concerned with the actual scalar values - compare the angles defined. I don't see a problem here. This is my concern with
Anglebeing a subclass ofQuantity; the latter appears to be defined by the scalar quantities and literal unit, while the former uses that only to determine the angle.eteq commentedon May 19, 2013
@demitri - I'm confused by what you said here. -10 degrees is not necessarily great than 5 degrees. An analogy might be the vector dot product of point A=(1,2) onto the x-axis and point B=(-2,1) onto the x-axis. I'd say A.x > B.x, because 1 > -2, even though the magnitude B.x is greater than A.x .
Put another way, it's inherently ambiguous whether or not an angle's sign is meaningful or not. As you said above, that could be an
AngularRotation, but it's not immediately obvious which one we mean in theAngleclass, and there's no way to make it unambiguous because of the imprecision of the word "Angle."So while I may be wrong, I think @astrofrog is suggesting just disabling it entirely. If we did that, we already have an
AngularSeparation, so we could just add anAnglesubclassAngularRotation. They could then be nearly-trivial subclasses that treat inequalities different (and possibly also do different things withbounds?).astrofrog commentedon May 19, 2013
@eteq - just to confirm, I am indeed suggesting disabling inequality comparisons on
Angleobjects.mdboom commentedon Sep 4, 2013
I believe this is resolved now. Reopen if not.
Merge pull request astropy#416 from astrofrog/simple-docs
Merge pull request astropy#416 from taldcroft/contribute-role