Wetter.com openwetter api

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
Antworten
grebnek
User
Beiträge: 4
Registriert: Mittwoch 2. Februar 2011, 21:53

Hallo zusammen,

dann trau ich mich auch mal hier im Forum etwas zuzeigen :oops:
Eigentlich nutze ich Python schon seid längerer Zeit, aber meistens sind es
nur kleinere Scripte für Dateioperationen.

Deswegen wollte ich mal etwas "sinnvolles" anfangen und bin irgendwie auf die API von Wetter.com gestossen.
Ich bin zwar selber noch nicht ganz zufrieden mit dem Code, hätte aber gerne bereits Feedback von euch.
Ist der Code Python Typisch? Sind Variablennamen evtl total verwirrend. PEP8?

Was mir direkt einfällt:
- AuthenticationError auslagern
- Exception für unbekannte Fehlermeldungen der Api?
- Zeilenlänge
- "_parse_json" verarbeitet kein json (evtl parse_respsonse)
...

https://gist.github.com/grebnek/4bfab143b399d66a88d0

Ich selber bin grad doch zu unsicher, werde wahrscheinlich den Code nur noch verschlimmern,
deswegen brauch ich mal ein Stoss in die Richtige Richtung. :lol:

Gruß grebnek
BlackJack

grebnek: Was meinst Du mit `AuthenticationError` auslagern?

Ich würde noch eine allgemeinere `Error`-Ausnahme einführen und `AuthenticationError` davon ableiten. Statt der superallgemeinen `Exception` könntest Du dann die Basis-`Error`-Klasse verwenden. So hat der Benutzer des Moduls die Möglichkeit geziehlt auf die Ausnahmen aus Deinem Modul zu reagieren.

PEP 8 könnte man bei der Namensgebung beachten.

Statt `xml` würde ich `use_xml` vorschlagen. Bei `xml` könnte man erwarten, dass hinter dem Attribut XML steckt. Zum Beispiel als Quelltext. Ich persönlich würde XML als Format ganz raus lassen.

Die Verwendung von ``''.join()`` in dem Programm ist ziemlich strange. In allen drei Fällen würde man eher ``+`` verwenden.

Gehört der Code ab dem Kommentar in `_parse_json()` wirklich in diese Methode? Ist der allgemein für alle möglichen Aufrufer von der Methode gültig/sinnvoll, oder gehört der eigentlich in verschiedene Aufrufer verteilt?
Benutzeravatar
snafu
User
Beiträge: 6740
Registriert: Donnerstag 21. Februar 2008, 17:31
Wohnort: Gelsenkirchen

Die Verwendung von `str.join()` um jeden Preis halte ich ebenfalls für wenig sinnvoll. `.join()` eignet sich eher dafür, ohne große Umstände eine Liste (oder gleichwertiges) von Strings zusammenzufügen. Dabei kann Code und - insbesondere wenn der Trenner kein leerer String ist - Denkarbeit gespart werden. Muss man für *zwei* Strings aber erst eine Liste erstellen und diese dann an `.join()` übergeben, dann hat man am Ende mehr getan als eigentlich sein musste. Will man mit der Laufzeit argumentieren, dann ist ein `.join()` für sehr wenige Strings sogar langsamer als das direkte Zusammensetzen. Lässt sich leicht nachprüfen:

Code: Alles auswählen

% python -m timeit 's1 = "foo"; s2 = "bar"' '"".join([s1, s2])'
1000000 loops, best of 3: 0.488 usec per loop
% python -m timeit 's1 = "foo"; s2 = "bar"' 's1 + s2'
1000000 loops, best of 3: 0.227 usec per loop
Grund dafür dürfte allein schon der Overhead für den Methodenaufruf sein, der bei solchen Mini-Aufgaben eben stärker ins Gewicht fällt. Das war jetzt aber mehr der Vollständigkeit halber gemeint. Ausschlaggebend ist in meinen Augen das Argument bezüglich Code / Lesbarkeit.

Übrigens, wenn man unbedingt `join`en will, dann möchte man vielleicht urljoin() aus dem `urlparse`-Modul verwenden... ;)
AlphaX2
User
Beiträge: 53
Registriert: Dienstag 28. Juni 2011, 10:42

Hallo,

ich hab neulich auch eine kleine Wetter.com API lib gebastelt, aber nicht sonderlich gut, muss da nochmal ran.
Und ich ruf für mein Widget nur allgemein einen festen Ort ab. Falls du einen Blick drauf werfen willst: https://github.com/AlphaX2/joshi-weathe ... therlib.py :) Allerdings bin ich mit dem was Wetter.com liefert eher unzufrieden, die Daten sind leider ziemlich ungenau.

Ich würde überlegen, ob Requests als seperates Modul sein muss, bei Wetter.com reicht ja auch die interne urllib2 aus, zumal du damit alle Plattformen erreichst und musst nicht gucken, ob Requests verfügbar ist, oder es sogar mit liefern. ;)

AlphaX2
Sirius3
User
Beiträge: 17754
Registriert: Sonntag 21. Oktober 2012, 17:20

Hallo AlphaX2,

was sollen die Funktionen in »LocationSearch.__init__«?
»WeatherForecast.__get_date_str« kann man komplett durch

Code: Alles auswählen

datetime.date.today().isoformat()
ersetzen, zumal der Kommentar falsch ist.

Das nackte »except« in »get_forecast« sollte durch passende Exceptions von urllib und json ersetzt werden.
Warum machst Du exception-Handling um danach per Boolean den Fehlerfall nochmals abzufragen?
AlphaX2
User
Beiträge: 53
Registriert: Dienstag 28. Juni 2011, 10:42

Hallo,

vielen dank für die Hinweise, wie gesagt das is nur sehr schnell zusammen geschrieben. Im __init__ die sind einfach falsch eingerückt und sollen eigentlich normale Methoden werden. Und das Exception-Handling sollte noch rein, auch is die doppelte Abfrage noch von einem vorherigem - anderem - Aufbau. :roll:

Das sich allerdings die ganze Methode mit der Zeile erstetzen lässt is cool. Das wusste ich nämlich gar nicht, hatte zwar fix gesucht, ob es was build-in gibt, aber nichts gesehen, also selber zusammen gebaut. Werd es demnächst mal überarbeiten, die Suche fehlt ja leider auch noch komplett.

AlphaX2
Antworten