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

when using a function to generate an angle for a point, it expects radians, not degrees #250

Open
burritojustice opened this issue Jun 25, 2018 · 8 comments

Comments

@burritojustice
Copy link

burritojustice commented Jun 25, 2018

https://github.com/tangrams/tangram-docs/blob/gh-pages/pages/draw.md#angle

when returning a value for angle with a function, it looks like it expect radians, not degrees.

            draw:
                mapzen_icon_library: # _points:
                    sprite: airport
                    collide: false
                    # angle: 180
                    angle: |
                        function(){
                            return 3.14/2
                            // return 180
                            }

returning 180 does not point south, while pi/2 does...

Note that passing degrees without a function, e.g. angle: 180, works as expected.

@nvkelso
Copy link
Member

nvkelso commented Jun 25, 2018

Is this an error in Tangram JS logic? Agree we should document the current state regardless, just wondering if there's another Tangram JS issue to file here? @meetar

@bcamper
Copy link
Member

bcamper commented Jun 25, 2018 via email

@burritojustice
Copy link
Author

burritojustice commented Jun 25, 2018

It looks like if you assign angle a number as a string, it assumes it's radians, but if you give it a number, it assumes degrees:

angle: "3.14" vs angle: 180 vs angle: "180"

So this could be something as simple as the function returning a string?

@bcamper
Copy link
Member

bcamper commented Jun 25, 2018 via email

@bcamper
Copy link
Member

bcamper commented Jun 26, 2018

Root issue is addressed in Tangram JS here: tangrams/tangram#652

Note this does NOT change static string behavior like angle: "90"... my opinion is that is incorrect syntax (there are valid string return values like auto, but a stringified number isn't one of them), and since YAML is generally good at type inference, we don't need to do any additional coercion here. That behavior was a useful clue as to why the function return value wasn't being parsed properly though.

@matteblair do you agree and does this reflect ES behavior?

@matteblair
Copy link
Member

The behavior you're describing sounds reasonable to me, I think it's unlikely that someone would quote a number literal for angle.

What I'm observing in Tangram ES now is that both number and string literals (e.g. 90 and "90") are accepted as valid degree values. Functions for the angle parameter only work in ES if the function returns the number as a string, which seems like a bug so I'll investigate that.

@bcamper
Copy link
Member

bcamper commented Jun 26, 2018

Got it. Yeah I also realized, the question with string representations of numbers is way more pervasive than this -- probably any (or many) parameters on the JS side that expect numbers. So I believe we should keep that as-is, with strict types (again I don't think this is a practical issue given YAML does this well), and we wouldn't want to just change behavior for this one parameter. Given differing JS vs. ES behavior, that seems like "undefined behavior" for going outside the documented value types for the parameter.

Sounds like if ES is updated to handle numbers returned as angles, we're in alignment on the expected/documented functionality anyway.

@matteblair
Copy link
Member

Yep, the issue with stringiness of scalars applies in other places as well.

The documentation for angle could also mention that a function can be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants