olgamirth / maxtemp

Max Temperature for the next Two Weeks
MIT License
2 stars 0 forks source link

Current commit #2

Open olgamirth opened 1 year ago

olgamirth commented 1 year ago

This is just the barebones MVP. I'm still working on it to add some "features" and make it poetry-enabled.

olgamirth commented 1 year ago

Why when line 32 is

temp = c2F('tMax')

do I get this error: Traceback (most recent call last): File "", line 1, in File "/var/home/dahlberg/PyBites/maxtemp/maxtemp/maxtemp.py", line 32, in temp = c2F('tMax') ^^^^^^^^^^^ File "/var/home/dahlberg/PyBites/maxtemp/maxtemp/maxtemp.py", line 8, in c2F return ((celsius * 1.8) + 32)


TypeError: can't multiply sequence by non-int of type 'float'

However, when line 32 is 
temp = c2F(tMax)
I don't get any errors?

Seems strange.
JnyJny commented 1 year ago

Well .. it's not strange :) strings can't be used in arithmetic expressions. When you pass in a string to c2F, your function evaluates to:

temp = ('tMax' * 1.8) + 32.0

I love that you wrote a function to convert Celsius to Fahrenheit, that's great. A couple of small corrections; first use a more descriptive function name. I get it, system administrators do a lot of typing and strive to minimize key strokes and automate things to save typing. Programmers also automate, but since the product we produce is read more times than it's written we have to be a little more verbose. It helps further down the line when we are puzzling over our code at 2am.

def celsius_to_fahrenheit(value: float) -> float:
     return (value * 1.8) + 32.0

The other style problem is your type annotation of the c2F return value. Instead of using a type float you are typing with a string "float". It's not a "technical" problem since your code will still run and static type checkers will likely accept your typing, but a python programmer conversant in type annotation will have to spend some time wondering why you typed it with a string. Our goal is to make the experience of reading our code be effortless like reading a sentence.

JnyJny commented 1 year ago

I'd like to see you encapsulate all the code in maxtemp/maxtemp.py into a function or set of functions to make it easier to get the data you are interested in. Something like:

# maxtemp/tomorrow.py

def get_forecast(api_key:str, location: tuple[float, float]) -> dict:
    """Returns a dictionary of forecast data from tomorrow.io."""
     ...

def high_temperatures(forecast: dict) -> list[float]:
    """List of forecasted high temperatures in Celsius."""
    ...

Then, you can write a file maxtemp/__main__.py that looks something like:

# maxtemp/__main__.py

from .tomorrow import get_forecast, high_temperatures

def main() -> None:
    api_key = open("path/to/api_key").read().strip()
    location  = (40.99460211262978, -77.56459180420583)

    forecast = get_forecast(api_key, location)

    max_temps = high_temperatures(forecast)

    print(max(max_temps))

if __name__ == "__main__":
    main()

Writing code this way is called top-down design, since we start at the "top" with the calls we'd like to make and then write the code going successively deeper to support the functions we wrote. The opposite, bottom-up design, is also useful but sometimes results in APIs that are more difficult or feel less natural than top-down designs.

JnyJny commented 1 year ago

In case it's not clear, you'd make a new file tomorrow.py in the maxtemp directory that has the new functions and then a new file maxtemp/__main__.py that uses a relative import to make the functions available. You'll end up with something like:

$ tree
.
├── LICENSE
├── README.md
├── maxtemp
│   ├── __init__.py
│   ├── __main__.py
│   ├── maxtemp.py
│   └── tomorrow.py
├── poetry.lock
├── pyproject.toml
└── tests
    └── __init__.py