2
\$\begingroup\$

Use case

The user is interested in supplying a timestamp of a particular moment in time. This python application will perform two astronomy calculations for a set list of planetary bodies:

  1. Calculate the longitude of the body's subpoint
  2. Calculate the boundary on the earth that separates the visible portion from the not visible portion, for an observer standing on the planet. Called the terminator

The user is interested in plotting these coordinates. The longitude of the body's subpoint is a float, and the user can choose to turn this into a line on a 2D map by transforming this into a (latitude, longitude) pair. The terminator is a set of geodetic coordinates (latitude, longitude), broken into two halves: halfA and halfB. This is relevant to the user.

All of these coordinates make up lines when plotted.

How this is used by the user

import datetime as dt

import pytz

from astrolib import calculatelines

if __name__ == "__main__":
   # Define the input parameteres
   timestamp_string = "2023-03-20 06:00:00"
   timestamp_format = "%Y-%m-%d %H:%M:%S"
   utc = "UTC"

   # Build an aware datetime object
   naive_observer_time = dt.datetime.strptime(timestamp_string, timestamp_format)
   aware_observer_time = pytz.timezone(utc).localize(naive_observer_time)
   
   # Calculate the results
   linescollection = calculatelines(aware_observer_time)

   # Get the data for the sun
   sun = linescollection.lines["Sun"]
   
   # Get the subpoint longitude for mars
   mars_subpoint_longitude = linescollection.lines["Mars"].subpoint_lng

   # Print the data for the entire set of results
   print(f"Moment in time: {linescollection.datetime}")
   for target, lines in linescollection.lines.items():
      print(f"Target: {target}")
      print(f"halfA: {lines.halfA}")
      print(f"halfB: {lines.halfB}")
      print(f"Subpoint longitude: {lines.subpoint_lng}")
      print(f"Anti subpoint longitude: {lines.anti_subpoint_lng}")

# $ python main.py
# Moment in time: 2023-03-20T06-00-00Z
# 
# Target: Sun
# halfA: [ ... ]
# halfB: [ ... ]
# Subpoint longitude: 91.898710654239
# Anti subpoint longitude: -88.101289345761
# 
# Target: Mercury
# halfA: [ ... ]
# halfB: [ ... ]
# Subpoint longitude: 94.88150140154595
# Anti subpoint longitude: -85.11849859845405
# 
# Target: Venus
# halfA: [ ... ]
# halfB: [ ... ]
# Subpoint longitude: 124.11636516090724
# Anti subpoint longitude: -55.883634839092764

The linescollection return object

from dataclasses import dataclass

import numpy as np

@dataclass
class Lines:
   target: str
   halfA: np.array
   halfB: np.array
   subpoint_lng: float
   anti_subpoint_lng: float

@dataclass
class LinesCollection:
   datetime: str
   lines: dict[str, Lines]

The calculatelines API

import datetime as dt

from astrolib import utils
from astrolib import config
from astrolib import astronomy
from astrolib.responses import ACGLines, ACGLinesCollection

def calculatelines(timestamp: dt.datetime) -> LinesCollection:
   utils.raise_exception_if_naive_time(timestamp)

   results = {}
   for target in config.CELESTIAL_INTERESTS.keys():
      subpoint_longitude = _calculate_subpoint_longitude(target, timestamp)
      anti_subpoint_longitude = _calculate_anti_subpoint_longitude(subpoint_longitude)
      halfA, halfB = _calculate_terminator_coordintes(target, timestamp, anti_subpoint_longitude)

      results[target] = ACGLines(target, halfA, halfB, subpoint_longitude, anti_subpoint_longitude)

   response = ACGLinesCollection(timestamp.strftime(config.ISO8061), results)
   return response

def _calculate_subpoint_longitude(target: str, time: dt.datetime) -> float:
   return astronomy.subpoint_meridian(target, time)

def _calculate_anti_subpoint_longitude(subpoint_longitude: float) -> float:
   return (subpoint_longitude % 360) - 180 # degrees

def _calculate_terminator_coordintes(target: str, time: dt.datetime, anti_subpoint_longitude: float):   
   terminator_coordinates = astronomy.terminator_great_circle(target, time, samples = config.SAMPLES)
   halfA_mask, halfB_mask = _get_masks(terminator_coordinates, anti_subpoint_longitude)
   halfA_coordinates = terminator_coordinates[halfA_mask]
   halfB_coordinates = terminator_coordinates[halfB_mask]

   return (halfA_coordinates, halfB_coordinates)

My Questions and Context:

The motivation for LinesCollection is the following:

  • I want the response from calculatelines() to have both the input parameters and the results
  • I want the results to be easily iterable
  • In the future if/when this is turned into a web API, perhaps the user can supply a list of bodies they are interested in. If they only supply one body, then they should still get the datetime in the response, but I don't want the response type of calculatelines to change from LinesCollection to Lines. The response should still have the same format in my opinion, and only the length of linescollection.lines should change
  1. Is there anyway to improve my API design? Something that feels awkward is linescollection.lines. It feels like this can be improved with the following code. Namely, calculatelines returns a Calculation object with the attribute results.

    calculation = calculatelines(aware_observer_time)
    sun = calculation.results["Sun"]
    mars_subpoint_longitude = calculation.results["Mars"].subpoint_lng
    
  2. Should Lines also have a datetime attribute? That way, given any Lines object for a particular body, the user has the complete info they need: the body name, the time, and the body's data. This feels cumbersome though. As long as the user has a LinesCollection they have the full set of data, but LinesCollection becomes a misnomer when the user only requests data for a singular body.

  3. Should calculate lines take a python datetime object or a string in ISO 8601 format and then the code will generate a python datetime object?

  4. Should I allow timezone inputs or should I push timezone responsibility onto the user and only allow inputs in UTC time?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

How this is used by the user

if __name__ == "__main__":

I am always pleased to see habitual use of a __main__ guard. But here, it does very little. Better to def main(): and make the guard do a one-line call to main.

We use such a guard so that other modules can safely import this one. For example, you might have a main() function calling some utility() function, and then a separate unit test module also wants to import and call into utility(). The guard lets that import complete successfully, without a print() or other side effects.

The absence of any def or class statements renders the current guard mostly moot.

redundant comments

   # Build an aware datetime object

This is a helpful comment, and I thank you for it.

Most of the other comments could be safely elided, as they say nothing beyond what the code has already said with eloquence and precision.

When describing a computation, we use code to explain the how and comments to explain the why.

Consider writing a very short make_zone_aware() helper, which would let you elide that "aware" comment. Indeed, in a codebase that quite thoroughly banishes naïve times, the notion of an "aware" distinction simply would not arise.

timestamp

class LinesCollection:
   datetime: str

I prefer an identifier of timestamp. As written, this suggests a dt.datetime object, which clearly a string is not.

We're not keeping track of the end user's favorite timezone, and that is fine, this codebase has a strong emphasis on "all UTC all the time!" which its users will soon come to know and love. That's a perfectly sensible way of representing instants in time.

If you do choose to keep this as a str rather than make it an aware dt.datetime, I would lobby for tacking on a final "Z". Either than or a final "+00:00". Then even if a maintenance engineer didn't read the docs that stress using UTC, we still can't accidentally misinterpret the meaning of the given timestamp.

(And I don't care whether a "T" or a SPACE separates YMD from HMS.)

lint

   halfA: np.array
   halfB: np.array

Pep-8 asks for names like half_a, whatever.

It would be nicer to spell it "lines_collection".

Also, I'm sorry that numpy type annotation is not yet easy to do. I hear it's improving and will be better in a future release. What you have there is helpful already.

nit, typo: def _calculate_terminator_coordintes

paranoia

   utils.raise_exception_if_naive_time(timestamp)

Wow, I love it, keep it up!

I have also seen CI/CD linting do an effective job of banishing naïveté from a large codebase.

meridian type

This is pretty interesting:

def _calculate_anti_subpoint_longitude(subpoint_longitude: float) -> float:
   return (subpoint_longitude % 360) - 180  # degrees

So the constraint is: -180 <= meridian < 180

Consider defining a custom return type for that, rather than float. The idea would be to make it very hard to accidentally manipulate a "300 degrees longitude" figure.

Astropy offers support for units.


type stability

If they only supply one body, then they should still get the datetime in the response, but I don't want the response type of calculatelines to change

Yes, I wholeheartedly agree. That's a good design principle.

naming

  1. ... improve my API design? Something that feels awkward is linescollection.lines.

I find that "collection", while not bad, is on the awkward side. Often we can find a suitable term, a collective noun, in the Business Domain. Here, given that we have celestial objects of interest, I propose that body_lines might fit -- it offers a Lines result per body.

(I do wish we didn't have a plural Lines, but it seems accurate and I don't see a better name.)

calculatelines returns a Calculation object with the attribute results.

I can't say I'm keen on that. It puts a strong emphasis on the whole business of calculating figures. I prefer to believe that the results are a true description of some aspect of the solar system, and worry about naming that.

  1. Should Lines also have a datetime attribute?

No, I see no motivation for that.

If some common usage or API winds up divorcing a Lines from its collection, we could revisit the issue, using new information. As written, the datastructure makes it very clear that all results are for matching timestamps.

LinesCollection becomes a misnomer when the user only requests data for a singular body.

I respectfully disagree. A container, or a collection, can have zero, one, or more items in it. It may be an "empty" or a "small" collection, and that is perfectly fine.

  1. Should calculate lines take a python datetime object or a string

My primary focus is the meaning of parameters should be unambiguous and clear. I'm partial to accepting an aware dt.datetime if that's convenient. If it's a str, then ending with Z or +00:00 suffices to make it clear.

  1. Should I allow timezone inputs or should I push timezone responsibility onto the user and only allow inputs in UTC time?

I have not yet seen any indication that end-user A needs their original timezone preserved, nor that communicating A's result object to end-user B would somehow be improved if B could readily tell what that timezone was. So "no", make it each user's responsibility.

We have an "astrolib subpoint library" which, given an instant in time, makes some lovely calculations. We could also have a "time utils library" with a few trivial helpers for going to/from UTC. And then we invite end users to compose the two libraries.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks so much for all the help. One last question: I've ultimately decided to name the two classes MapLines and TargetLines, where the user can do MapLines.targets["Sun"]. I am trying to decide between MapLines/TargetLines and MapCoordinates/TargetCoordinates. I am leaning towards coordinates because that more accurately represents the data. One gripe is that I feel like "Map" has a 2D connotation, and I would like to imply that these coordinates can be plotted in 3D, but not sure if I'm overthinking that lol. Would like to get your opinion on this final naming schema \$\endgroup\$
    – Hunter
    Mar 13 at 22:19
  • \$\begingroup\$ Meh, IDK, "naming is hard!". Go with your gut. I confess a slight preference for "Lines" vs "Coordinates" just because it is fewer syllables, fewer letters, but "Coords" is a fair compromise on that front. (If we start wandering through the mathematical lexicon I'm guessing that "Locus" will be rejected out of hand...) \$\endgroup\$
    – J_H
    Mar 13 at 23:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.