skyfielders / python-skyfield

Elegant astronomy for Python
MIT License
1.41k stars 212 forks source link

Cannot compare Distances #818

Open sligocki opened 1 year ago

sligocki commented 1 year ago

It would be nice if we could compare Distance objects to math.inf for convenience in algorithms like finding the closest object:

  min_dist = math.inf
  closest = None
  for dest in dests:
      position = source.at(time).observe(dest)
      distance = position.distance()
      if distance < min_dist:
        min_dist = distance
        closest = dest

Right now this fails in Python 3.10: TypeError: '<' not supported between instances of 'Distance' and 'float'

sligocki commented 1 year ago

Actually, I see that Distance objects are not even coparable to each other at all:

TypeError: '<' not supported between instances of 'Distance' and 'Distance'

sligocki commented 1 year ago

So the workaround here is to use position.distance().au instead of position.distance() directly. This allows comparing distances to each other and to math.inf. Seems like it would be nice to be able to compare Distance objects directly though.

brandon-rhodes commented 1 year ago

I'm catching up on Skyfield issues that arrived over the winter! I waited a bit to answer this one to see if my initial reaction still held, which is that having users compare numbers like .au is better than starting to festoon all of the Skyfield unit classes with all the math methods that would be necessary to support comparisons.

Given that supporting comparisons would require something like five new methods on the Unit class, whereas it works just fine if users compare .au values, I'm at this point still not inclined to add code to Skyfield to support direct unit comparison.

Also, I guess I worry that then people would want to do all kinds of math with Distance objects, like adding and subtracting them, and that would require yet more dunder methods.

Hmm. Maybe I should go write one, just to see how terrible it would be.

sligocki commented 1 year ago

Hi @brandon-rhodes , I'm super new here, so don't let my whims drive your implementation plan too much :) Consider this to simply be a comment from someone new coming in and being confused.

I do still think it's confusing that you can't compare calls to distance() directly to each other. But maybe that's a one-time confusion and then with a bit of research others (like me) will realize you have to call .au or some similar method to convert them to floats.

There is a "units" python package: https://pypi.org/project/units/ that might take care of a lot of the implementation clutter you are talking about ... but I don't know if you want to convert your whole system over to the specific ways that package works :) Up to you.

Otherwise another partial solution could be to implement comparison overrides with NotImplementedError or TypeError that would include a small description of how users should actually be doing comparison ... but maybe implementing that is roughly as hard as just implementing the comparisons directly, so dunno, heh.