Open hugoledoux opened 1 year ago
Thanks for the feedback. (Also I love your work! I read through your Computational Terrain book a while ago and it was very informative).
- you point to suncalc-js as a reimplementation, but the function there takes lat-long as input, and you take long-lat... 100% prefer long-lat, but this is somewhat confusing.
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.
- the azimuth given is not CW from the North (as an azimuth is usually), but from the South and CW, and in radians. This is not explained anywhere, and I had to reverse-engineer this and then saw it at mourner/suncalc#sun-position, but then you think: if the author swapped lat-long, maybe they also corrected the azimuth from the South.
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 = datetime.datetime.utcnow()
>>> now
datetime.datetime(2023, 6, 28, 22, 48, 29, 897651)
>>> get_position(now, 138.7, -34.7)
{'azimuth': 1.149960651953923, 'altitude': -1.1743770134613025}
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