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

add irrational constant ° = pi/180, for converting degrees to radians #16765

Closed
wants to merge 1 commit into from

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Jun 4, 2016

This allows you to have a constant like 30° yield the angle in radians, as discussed in #16760.

On the one hand, this syntax is almost too cute. On the other hand, it's not clear what else one could do with the ° identifier, and wanting to define angular constants in degrees is undeniably common and useful. (Declaring it to be a special Degree type, while it would allow for more error checking, seems like it would get too intrusive ... too many functions would need new methods if we introduced a new numeric type.)

The main other option discussed in #16760 would be to define ° as a postfix operator equivalent to deg2rad, probably with high precedence. This is also workable, but would disallow ° in identifiers, and it's not at all clear to me that an expression like φ° should be a function call rather than just a variable name.

@stevengj stevengj added the speculative Whether the change will be implemented is speculative label Jun 4, 2016
@simonbyrne
Copy link
Contributor

-1 from me: since sin(30°) != 0.5, yet we have the perfectly good sind(30) == 0.5. It seems absurd to create a second syntax that is objectively worse. Are there any other use cases?

I think I would prefer the Degree type proposed by Stefan in #16760 (comment) but perhaps that's better left to a units package.

@stevengj
Copy link
Member Author

stevengj commented Jun 4, 2016

@simonbyrne, if someone writes sin(30°) and cares about accuracy, they might as well just write 0.5 and skip the function call. Furthermore, we only have sind and cosd, but there are lots of other trig functions, and lots of other circumstances where one uses an angle.

The problem with a Degree type is the same problem that we had when im was an ImaginaryUnit type: at some point you give up on defining methods for the new type, and then it fails unexpectedly. Besides, radians and degrees aren't really units in the conventional sense (because they are both dimensionless/scale-invariant).

@eschnett
Copy link
Contributor

eschnett commented Jun 4, 2016

° should have the same status as the constant π.

Regarding @simonbyrne's argument: if you write sin(π/2), this will also be less accurate than sinpi(0.5), yet we still support π as irrational constant.

@@ -599,4 +604,3 @@ As ``BigInt`` represents unbounded integers, the interval must be specified (e.g
Create an array of the size ``jumps`` of initialized ``MersenneTwister`` RNG objects where the first RNG object given as a parameter and following ``MersenneTwister`` RNGs in the array initialized such that a state of the RNG object in the array would be moved forward (without generating numbers) from a previous RNG object array element on a particular number of steps encoded by the jump polynomial ``jumppoly``\ .

Default jump polynomial moves forward ``MersenneTwister`` RNG state by 10^20 steps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is your editor doing this? check that genstlib doesn't put it back

@ararslan
Copy link
Member

ararslan commented Jun 5, 2016

@eschnett Granted, π has uses outside of angles, whereas ° wouldn't if we declare it to be π/180.

@iamed2
Copy link
Contributor

iamed2 commented Jun 5, 2016

While it may be useful in some cases, I think having application of a degree symbol yield a numeric value which is not in degrees is counter-intuitive. Better to leave this to package developers (who may have other uses, like temperature or music notation).

@andyferris
Copy link
Member

Overall, I really like the idea of defining ° as something. In fact I've been thinking about implementing something like this in my own packages, but am reluctant precisely because it might be useful to a variety of packages and probably should go into Base.

But I feel the proposal that ° wraps a number in the Degree immutable type for multiple dispatch to take care of is probably better and more expressive, so long as we can work out the complications and fallout (e.g. what is Degree * Degree (error or solid angle?), how do other (non-trigonometric) functions in Base interact with Degree, and so on).

Better to leave this to package developers (who may have other uses, like temperature or music notation).

Indeed. We shouldn't forbid °C and °F as well-defined symbols by making ° an operator. It can easily be a singleton with juxtaposition for multiplication (both 30° or (x)° aren't too bad, the latter even gives the feeling that x is being "wrapped" in by the degree, which aligns nicely with my preferred proposal).

@toivoh
Copy link
Contributor

toivoh commented Jun 6, 2016

I also think that it would seem better to preserve the identity of degrees (through a Degrees type or similar).

Sure, it's more work, but we do want Base to play more nicely with units #15394, and as far as I understand, it is basically that work which is needed.

@stevengj
Copy link
Member Author

stevengj commented Jun 6, 2016

Okay, seems like the consensus is against having this in Base.

@stevengj stevengj closed this Jun 6, 2016
@StefanKarpinski
Copy link
Member

I'm all for the syntax being parsed as º(x) so that someone can define the º function as they want.

@stevengj
Copy link
Member Author

stevengj commented Jun 6, 2016

I'm worried that ° is too potentially useful in identifiers (e.g. for °F and °C units constants) to parse it as an operator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
speculative Whether the change will be implemented is speculative
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants