-
Notifications
You must be signed in to change notification settings - Fork 9
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
lat-long vs long-lat && azimuth starting from South is just confusing #9
Comments
Thanks for the feedback. (Also I love your work! I read through your Computational Terrain book a while ago and it was very informative).
I did switch to long-lat ordering. All of the examples and API docs in the README have long before lat, so I would think this reordering wouldn't be too hard to find.
To be honest, I was making this package as a quick port and didn't know how azimuths were usually measured. I'm definitely not an expert on sun position algorithms; just wanted to port suncalc.js and have it be vectorized. Not sure if a documentation change or a code change would be better here? Is it worth a breaking change? |
Glad my work is useful to you. Currently using the library in the course for which the book was written, it was by far the simplest way to get the sun's position: https://3d.bk.tudelft.nl/courses/geo1015/hw/02/ My point was more: since you swapped (rightfully so!) lat-long, then just state it in the docs explicitly? Because when I got the azimuth I was lost (it makes no sense!), and then went to the suncalc-js docs and then saw lat-long and got super confused and started to test everything again to ensure it works. But hey, no big deal, the library seems to work otherwise (although all tools online give slightly different values for the sun, which is slightly annoying: which one is the best?) |
Is there some similar peculiarity with the altitude values as well?
Now is 'now', and I currently have the sun shining in a window at me, so it's definitely above the horizon! |
Very useful package, but 2 things that should be made clear in the docs IMO:
Hope this helps
The text was updated successfully, but these errors were encountered: