AndrewAnnex / SpiceyPy

SpiceyPy: a Pythonic Wrapper for the SPICE Toolkit.
MIT License
395 stars 84 forks source link

sincpt unpacks 3 values #461

Closed kconnour closed 1 year ago

kconnour commented 1 year ago

Describe the bug The return value of sincpt doesn't match the documentation... or seemingly the code strangely enough. It appears that this function should return 4 values but instead it only returns the first 3.

Expected behavior I expect this function to return a tuple with 4 values: an array, a float, an array, followed by a boolean. Instead, I can only get this function to return a tuple with 3 values: an array, a float, and an array. I don't care about the boolean (or the last 2 values), but this means that this code (that I would expect to work) does not:

spoint, _, _, _ = spice.sincpt('Ellipsoid', target, et, frame, abcorr, observer, frame, bin_vector)

It raises this: ValueError: not enough values to unpack (expected 4, got 3)

However, trying to unpack 3 values raises no error and the values look good.

To Reproduce

target = 'Mars'
observer = 'MAVEN'
abcorr = 'LT+S'
body = 499  # Mars IAU code
frame = 'IAU_Mars'
et = 521320703.35254663 
bin_vector = [0.64829554 0.20495633 0.14674814]

spoint, _, _, _ = spice.sincpt('Ellipsoid', target, et, frame, abcorr, observer, frame, bin_vector)

I can't tell you more about the kernels I'm using. I'm using code someone else on the team wrote to load them in and there are a few thousand of them loaded.

Desktop (please complete the following information):

Additional context Add any other context about the problem here, screenshots

AndrewAnnex commented 1 year ago

@kconnour thanks for the issue, so the 4th parameter is a boolean "found" flag which is swallowed up by the spice_found_exception_thrower decorator which changes the effective return type of the function by returning only the first three parameters, see https://spiceypy.readthedocs.io/en/main/_modules/spiceypy/spiceypy.html#spice_found_exception_thrower. This is currently the default behavior for all functions that return a "found" flag as the last parameter of the return (eg bodn2c). That does make the doc a little deceptive if you aren't aware of that behavior, but the behavior has been in spiceypy for a number of years now.

To see this you can try the call using the context manager for the checker, modifying your example like so:

target = 'Mars'
observer = 'MAVEN'
abcorr = 'LT+S'
body = 499  # Mars IAU code
frame = 'IAU_Mars'
et = 521320703.35254663 
bin_vector = [0.64829554 0.20495633 0.14674814]
with spice.no_found_check():
    spoint, _, _, _ = spice.sincpt('Ellipsoid', target, et, frame, abcorr, observer, frame, bin_vector)

which shouldn't raise an exception.

Likely your colleague has disabled the decorator somewhere else in their code by calling spice.found_check_off() which disables the decorator without using the context manager, or else they were just going off of the docstring directly.

I could probably think about a better way to convey the return types for these functions but let me know if my explainer above makes sense.

kconnour commented 1 year ago

This makes sense. Thanks for the quick and clear response! I suppose the real issue for me here is that I found the documentation misleading when looking at the return types, but I should be set for now.

AndrewAnnex commented 1 year ago

okay great, thanks @kconnour. I'll keep this issue open for now as I consider ways to improve the docs but for those reading this wasn't a bug hence the labels I've applied to the issue.

AndrewAnnex commented 1 year ago

closing the issue for now as it was fixed in #462