5
\$\begingroup\$

I have a project where the user supplies a date, time, and timezone, and my code will calculate a bunch of coordinates on the Earth. There are four sets of coordinates setA, setB, setC, setD. Coordinates setA and setB make up a great circle around the Earth. Likewise for coordinates setC and setD. The user is interested in plotting these coordinates in a 2D mapping system like Google Maps.

CoordinatesCalculator.py

import datetime as dt

import numpy as np

from . import utils
from . import config
from . import astronomy

class CoordinatesCalculator:
   def __init__(self) -> None:
      self.results = None

   def calculate(self, timestamp: dt.datetime, longitude: float, latitude: float) -> dict[str, dict[str, np.array]]:
      self.results = {}

      for name, data in config.CELESTIAL_INTERESTS.items():
         # Calculate coordinates
         a, b = sets_A_and_B()
         c = setC()
         d = setD()

         self.results[name] = {
            "setA": a,
            "setB": b,
            "setC": c,
            "setD": d
         }

      return self.results
   
   def set_C(self, target: utils.CelestialTarget, time: dt.datetime) -> float:
      return astronomy.subpoint_meridian(target, time)
   
   def set_D(self, c: float) -> float:
      return (c % 360) - 180 # degrees
   
   def sets_A_and_B(self, target: utils.CelestialTarget, time: dt.datetime):      
      return astronomy.terminator_great_circle(target, time, samples = config.SAMPLES)

   def print_sets_C_and_D(self) -> None:
      ...
         
   def export_csv(self, targets: list[str], latitude_first = False) -> None:
      ...
   
   def plot2D(self, targets: list[str]) -> None:
      ...

How this is used by the user

import datetime as dt

import pytz

from .CoordinatesCalculator import CoordinatesCalculator

if __name__ == "__main__":
   timestamp_string = "2021-06-20 06:00:00"
   tz = "Europe/London"
   latitude = -113
   longitude = 46

   # Build an aware datetime object
   naive_observer_time = dt.datetime.strptime(timestamp_string, "%Y-%m-%d %H:%M:%S")
   aware_observer_time = pytz.timezone(tz).localize(naive_observer_time)
   
   # Calculate the results
   calculator = CoordinatesCalculator()
   results = calculator.calculate(aware_observer_time, longitude, latitude)

   # Print, plot, and export the results
   calculator.print_sets_C_and_D()
   calculator.plot2D(["Sun", "Moon", "Mars"])
   calculator.export_csv(["Sun", "Moon", "Mars"])

Return dictionary

results = {
   "Sun": {
      "setA": np.array([]),
      "setB": np.array([]),
      "setC": np.array([]),
      "setD": np.array([])
   },
   "Moon": {
      "setA": np.array([]),
      "setB": np.array([]),
      "setC": np.array([]),
      "setD": np.array([])
   },
   "Mars": {
      "setA": np.array([]),
      "setB": np.array([]),
      "setC": np.array([]),
      "setD": np.array([])
   }
}

My Questions

  1. The CoordinatesCalculator class doesn't really have a state, should this just become a function?

  2. Should my return dictionary also have parameters that the user inputted? I kind of want some object that contains everything about the calculation (e.g. the time, date, time zone, and the resulting coordinates). The issue with the following is it makes it weird to iterate over if the user wants to just iterate over the coordinates data.

    results = {
       "Date": "2021-06-20",
       "Time": "06:00:00",
       "Timezone": "Europe/London",
       "Sun": {
          "setA": np.array([]),
          "setB": np.array([]),
          "setC": np.array([]),
          "setD": np.array([])
       },
       "Moon": {
          "setA": np.array([]),
          "setB": np.array([]),
          "setC": np.array([]),
          "setD": np.array([])
       }
    }
    
  3. The CoordinatesCalculator is handling functions that belong to the results, such as export_csv and plot2D. Should I make the results its own class Results that then has the export_csv and plot2D? This would satisfy the itch for an object that contains everything about the calculation. Something like:

    class Results:
       def __init__(self, date, time, timezone, setA, setB, setC, setD):
          self.date = date
          self.time = time
          self.timezone = timezone
          self.setA = setA
          self.setB = setB
          self.setC = setC
          self.setD = setD
    
       def print_sets_C_and_D(self):
          pass
    
       def export_csv(self):
          pass
    
       def plot2D(self):
          pass
    
       def to_json(self):
          pass
    
  4. If I wanted to turn this into an API, does it suffice to just have a Results.to_json() function? Maybe the JSON results would have the structure, so that the user can iterate over the results in a clean way:

    {
       "date": date,
       "time": time,
       "timezone": timezone,
       "results": {
          "Sun": {
             "setA": [],
             "setB": [],
             "setC": [],
             "setD": []
          },
          "Moon": {
             "setA": [],
             "setB": [],
             "setC": [],
             "setD": []
          }
       }
    }
    
\$\endgroup\$
1
  • \$\begingroup\$ Instead of requiring the user to supply the timepoint in three different parameters, perhaps accept a single UTC timepoint, and suggest ways to convert from other things (local time or separate day/time). \$\endgroup\$ Mar 11 at 18:13

1 Answer 1

7
\$\begingroup\$
  1. Should CoordinatesCalculator just become a function?

Yes. It wants to be a module offering some functions that will calculate, plot, export to CSV and so on.

Use an _ underscore prefix for any private helper functions that a user would have no need of calling directly. That way we offer a relatively simple Public API for each new user to learn about.

  1. Should my return dictionary also have parameters that the user inputted?

Yes.

The issue ... is it makes it weird to iterate over if the user wants to just iterate over the coordinates data.

I respectfully disagree. We see this all the time in e.g. RESTful JSON APIs.

for name, coords in result['Sun']:
    assert name.startswith('set')
    ...

Or to examine more coordinates:

celestial_bodies = ["Sun", "Moon", "Mars"]
for body in celestial_bodies:
    for name, coords in result[body]:
        assert name.startswith("set")
        ...

Alternatively, define a helper function that dynamically discovers the relevant set of celestial bodies by probing the result, and iterate over names returned by the helper.

You could also define variant API endpoints, variant function names, accept a set of desired celestial bodies, and so on. But that sounds like putting the wrong amount of complexity in the API, given that the items are small to transport across TCP and are not time consuming to produce. We're probably better off to just let the client pick out its favorite values. It can readily define a helper that extracts its favorite subset, and then a different client can similarly define a helper for its slightly different needs. Such an approach will tend to reduce version churn in the server's API, as the third or fourth client explores additional regions of the powerset of celestial bodies.

An aspect of the API which is very weird is to see date + time broken up. Offer it as a single ISO-8601 timestamp.

Also, consider normalizing the user's input to UTC, so your timestamps all end with "Z". Do hang onto the original timezone name as a separate field.

I imagine that your astronomy module uses TAI (or TT) on the inside, rather than UTC. Consider including both in the result dict, or at least include the number of seconds they differ by for the given timestamp.

  1. Should I make the results its own class Result that then has the export_csv and plot2D?

Yes, a Result class would be lovely.

Consider making it a dataclass.

  1. If I wanted to turn this into a [web] API, does it suffice to just have a Results.to_json() function?

Yup, pretty much.

You may find .asdict() convenient.

Or use a @dataclass_json decorator.

It is a fact of life that Things Change. This includes software, and the APIs defined by software. It is possible that your webclient caller is already aware of which version of your API they're accessing, for example because the URI started "https://api.example.com/v2/calculate". Alternatively you might wish to insert a SemVer k: v pair like "version": "2.0.1" so the client will know what keys to expect, and what semantics are imposed on the values. It is customary to bump the version after implementing a bugfix or offering new features.

You solicit a (lat, long) coordinate. Sometimes users supply 53 bits of FP significand, with most of the low order bits being meaningless. Consider deliberately rounding to ~ 6 decimal places, as one-meter resolution on Earth's surface likely suffices for your purposes.

\$\endgroup\$
7
  • \$\begingroup\$ Sorry what I meant about the dictionary being weird to iterate over is when several bodies are included. For example, see the "Return Dictionary" section, where there are multiple bodies. It's easy to iterate over that like body, coordinates in results.items(), but if there were to be a "timestamp", and "timezone" key in the results dictionary, then body, coordinates in results.items() wouldn't work. But perhaps my amendment is still nil to your suggestion that this is common practice in RESTful APIs \$\endgroup\$
    – Hunter
    Mar 12 at 0:20
  • \$\begingroup\$ Ok, I get your concern. I will flesh out my remark a bit more to address it. I had based it just on the example result record, which only included a single celestial body, the Sun. \$\endgroup\$
    – J_H
    Mar 12 at 0:23
  • \$\begingroup\$ Sweet thanks. I've updated the examples to include two bodies. My proposed solution would be to stuff all they body, coordinates key, value pairs into a data section inside the results dictionary (see the example JSON object in question 4). \$\endgroup\$
    – Hunter
    Mar 12 at 0:27
  • \$\begingroup\$ Well, it's your API, so you can define the rules however you want. Personally, I tend to find a key which is literally labeled data to be on the annoying side, as it is hopelessly vague and it just pushes the bits I care about One More Level deeper down in the nesting. OTOH if we have keys named data_for_weekly_report and data_for_monthly_report, then the level of nesting is actually pulling its weight, organizing the data and making it self descriptive. Your experiences in writing client code will inform your server-side design decisions. Make things easy for the client. \$\endgroup\$
    – J_H
    Mar 12 at 0:34
  • \$\begingroup\$ This is my first time trying to design an API for something that will be stuck in the future, so sorry if this question appears to be too in the weeds of this stuff. What might be a nice way to design the Results class. What i'm thinking is CalculatorResults(iso8601: string, collection: list[Coordinates]) is the response from the calculator, and a Coordinates class looks like: Coordinates(body_name: str, setA: list, setB: list, setC: float, setD: float). The idea of "set" here is incorrect for C and D, they're only for this example. for coordinates in response.collection could work \$\endgroup\$
    – Hunter
    Mar 12 at 17:57

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.