Navigation Menu

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

Method 'Angle.dstr()' does not return a list if Angle has multiple values #527

Closed
miccoli opened this issue Jan 2, 2021 · 3 comments
Closed

Comments

@miccoli
Copy link

miccoli commented Jan 2, 2021

Given this initialisation

from skyfield.units import Angle
import numpy as np

a = Angle(degrees=np.arange(0,180, 30))

method a.dstr() returns the string

'6 values from 00deg 00\' 00.0" to 150deg 00\' 00.0"'

while I would expect

['00deg 00\' 00.0"',
 '30deg 00\' 00.0"',
 '60deg 00\' 00.0"',
 '90deg 00\' 00.0"',
 '120deg 00\' 00.0"',
 '150deg 00\' 00.0"']

The only way I found to obtain a list of strings is the rather clumsy

[Angle(degrees=i).dstr() for i in a.degrees]

BTW,

for i in a:
    pass

raises

ValueError: choose a specific Angle unit to iterate over

Instead of iterating over this Angle object, try iterating over one of
its unit-specific arrays like .degrees, .hours, or .radians, or else over
the output of one of its methods like .hstr(), .dstr(), .arcminutes(),
.arcseconds(), or .mas().  For all of the possibilities see:
https://rhodesmill.org/skyfield/api-units.html#skyfield.units.Angle

confirming that a.dstr() should be an iterable of strings.

brandon-rhodes added a commit that referenced this issue Jan 2, 2021
This replaces a behavior that was contributed back in, wow, 2015 — crazy
times!  Also, let’s test this behavior in unit tests for the first time.
@brandon-rhodes
Copy link
Member

Great idea! I must never call those methods myself, as I didn't realize that wasn't already their behavior.

I've just landed an improvement. You can try it out with:

pip install https://github.com/skyfielders/python-skyfield/archive/master.zip

Let me know whether that fixes the problem and brings their behavior into line with the ValueError text. Thanks!

@miccoli
Copy link
Author

miccoli commented Jan 2, 2021

Yes, commit ce9411c implements the expected behaviour. Thanks.

@brandon-rhodes
Copy link
Member

Great! I'll be sure to add it to the changelog before the next release, which hopefully will be early to mid January.

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

2 participants