python style

Zasady dotyczące pisania programów w Pythonie

W przypadkach, które nie są sprecyzowane poniżej stosujemy się do PEP8.

Styl

• Nie mamy sztywnego ograniczenia długości linii, ale zalecane jest nie więcej niż 120 znaków.
• Porównujemy eksplicite.
  • 🗸 if value != 0:
  • 🗸 if value is not None:
  • 🗸 if value not in ["", [], None]:
  • 🗸 if len(items) > 0:
  • 𐄂 if value:
  • 𐄂 if items:
• Eksplicite zwracamy wartość z funkcji za pomocą return, chyba że funkcja z założenia nie ma zwracać wartości (tzn. wszystkie ścieżki kończą się zwróceniem None).
• Unikamy wartości domyślnych innych niż None lub pusty string.
• Unikamy umieszczania w modułach kodu, który wywołuje się automatycznie w momencie importu.
  • W skryptach cały kod, który wykonuje się na poziomie modułu zabezpieczamy za pomocą if __name__ == "__main__": tak aby wciąż dało się je importować jako moduły bez automatycznego wykonania ich kodu.

Zewnętrzny kod

• Nie commitujemy nienapisanego przez siebie kodu do repozytorium. Biblioteki powinno być wymienione jako zależności i instalowane za pomocą pip.
  • Jeżeli biblioteka nie jest dostępna jako pakiet pythonowy albo repozytorium na githubie, tworzymy na githubie własnego forka i instalujemy stamtąd.
• Nie kopiujemy kodu. Każdy kawałek kodu zbyt mały by mógł być użyty jako biblioteka powinien zostać zrozumiany i napisany od podstaw zgodnie z naszym stylem, a znane problemy naprawione. Jeżeli kod znajduje się w repozytorium to jest traktowany jak nasz kod i nie jest wyłączony ze spełniania wymagań opisanych w tym dokumencie.
• Nie zostawiamy zakomentowanego kodu. Jeżeli istnieje bardzo dobry powód by taki kod zostawić musi być on przynajmniej opisany: dlaczego został wykomentowany i w jakich warunkach można będzie go usunąć.

Nazewnictwo

• Nie stosujemy skrótów w nazwach.
• Jednoliterowe nazwy zmiennych dopuszczalne są jedynie w jednolinijkowych pętlach.
• Unikamy używania ponownie tej samej zmiennej do przechowywania innych danych.
• Nazewnictwo metod w testach: test_that_<co jest testowane>_should_<oczekiwany efekt>().
  • 🗸 test_that_send_view_should_accept_a_valid_message_and_respond_with_http_200()
  • 𐄂 test_send_view_should_accept_a_valid_message_and_respond_with_http_200()
  • 𐄂 test_send_view_accept_a_valid_message_and_respond_with_http_200()
  • 𐄂 test_send_view()
• Nazwa zmiennej powinna być rzeczownikiem.
  • 🗸 value
  • 🗸 invalid_response
  • 🗸 decoded_message
  • 𐄂 check_message
• Nazwa funkcji powinna być czasownikiem.
  • Unikamy nazw zaczynających się od check_.
  • 🗸 get_response()
  • 🗸 send()
  • 🗸 prepare_transaction_data()
  • 𐄂 check_response()
  • 𐄂 message()

Zawijanie, wcięcia, nawiasy

• Importy: jedna nazwa w jednej linijce, uporządkowane alfabetycznie.
• Na końcu list, list argumentów, słowników, itp. zawiniętych na wiele linijek dodajemy przecinek.
  • 🗸 {'key1': 'value1', 'key2': 'value2'}
  • 🗸

    {
        'key1': 'value1',
        'key2': 'value2',
    }

  • 𐄂 {'key1': 'value1', 'key2': 'value2',}
  • 𐄂

    {
        'key1': 'value1',
        'key2': 'value2'
    }

• Kodu nie wyrównujemy tak aby podobne elementy się pokrywały.
  • 🗸

    force_task_to_compute_message = Message.objects.filter(task_id = message.task_to_compute.compute_task_def['task_id'], type = message.ForceReportComputedTask.TYPE)
    previous_ack_message = Message.objects.filter(task_id = messae.task_to_compute.compute_task_def[id'], type = message.AckReportComputedTask.TYPE)
    reject_message - Message.objects.filter(task_id = message.task_to_compute.compute_task_def['task_id], type = message.RejectReportComputedTask.TYPE)

  • 𐄂

    force_task_to_compute_message = Message.objects.filter(task_id = message.task_to_compute.compute_task_def['task_id'], type = message.ForceReportComputedTask.TYPE)
    previous_ack_message          = Message.objects.filter(task_id =  messae.task_to_compute.compute_task_def[id'],       type = message.AckReportComputedTask.TYPE)
    reject_message                - Message.objects.filter(task_id = message.task_to_compute.compute_task_def['task_id'], type = message.RejectReportComputedTask.TYPE)

  • 🗸

    @unique
    class State(Enum):
        PENDING = 'P'
        IN_PROGRESS = 'I'
        SUCCESS = 'S'
        ERROR = 'E'
        INTERRUPTED_BY_USER = 'U'

  • 𐄂

    @unique
    class State(Enum):
        PENDING             = 'P'
        IN_PROGRESS         = 'I'
        SUCCESS             = 'S'
        ERROR               = 'E'
        INTERRUPTED_BY_USER = 'U'

• W definicjach zawiniętych na wiele linijek nawias zamykający powinien mieć ten sam poziom wcięcia co cała instrukcja/definicja.
  • 🗸

    User.objects.filter(
        name = "John",
        active = False,
    )

  • 🗸

    Stuff.objects.annotate(
        adds = Count(
            Case(
                When(
                    token__operation = "A",
                    then = F("token__id"),
                )
            ),
            distinct = True,
        )
    ).annotate(
        difference = F("add") - F("delete"),
    ).filter(
        Q(useless_column_with_a_very_long_name__lt = 0) |
        Q(useless_column_with_a_very_long_name__gt = 1),
    ).exists()

  • 𐄂

    User.objects.filter(
        name = "John",
        active = False,
        )

  • 𐄂

    User.objects.filter(
        name = "John",
        active = False)

  • 𐄂

    User.objects.filter(name = "John",
                        active = False)

• Nie dodajemy spacji wokół operatora = przy nazwanych argumentach.
  • 𐄂 def send_message(code = 0, default_message = None)
  • 𐄂 response = send_message(code = 200, default_message = "Success")
  • 𐄂

    response = send_message(
        code            = 200,
        default_message = "Success",
    )

  • 🗸 def send_message(code=0, default_message=None)
  • 🗸 response=send_message(code=200, default_message="Success")
  • 🗸

    response=send_message(
        code=200,
        default_message="Success",
    )

  • 𐄂

    response=send_message(
        code           =200,
        default_message="Success",
    )

• Unikamy escape'owania cudzysłowów w stringach jeżeli da się tego uniknąć.
  • 🗸 "That's an error. Let's fix this 'issue'."
  • 🗸 'That\'s an error. Let\'s fix this "issue".'
  • 🗸 "That's an error. Let's fix this \"issue\"."
  • 𐄂 'That\'s an error. Let\'s fix this \'issue\'.'

Inne

• Zachowujemy nawiasy wokół krotek nawet kiedy jest to opcjonalne.

  • 🗸 pair = (key, value)
  • 🗸 (key, value) = get_pair()
  • 🗸 return (key, value)
  • 𐄂 key, value = get_pair()
  • 𐄂 return key, value

• W definicjach funkcji przekraczających 120 znaków łamiemy linię tak aby każda zawierała jeden argument:

  • 🗸

    def store_message_and_message_status(
        golem_message_type: int, 
        task_id str, 
        raw_golem_message: bytes, 
        status = None, 
        delivered: bool = False
    ):

  • 𐄂

    def store_message_and_message_status(golem_message_type: int, task_id: str, raw_golem_message: bytes, status = None, delivered: bool = False):

  • 𐄂

    def store_message_and_message_status(
        golem_message_type: int, task_id: str, 
        raw_golem_message: bytes, 
        status                    = None, 
        delivered:         bool   = False
    ):

  • 𐄂

    def store_message_and_message_status(
        golem_message_type: int, 
        task_id: str, 
        raw_golem_message: bytes, 
        status   = None, 
        delivered: bool = False
    ):

  • 𐄂

    def store_message_and_message_status(
        golem_message_type: int, task_id, 
        raw_golem_message: bytes, 
        status             = None, 
        delivered:         bool = False
    ):

Zasady pisania poprawnego kodu

Funkcje

• #TODO: rozbudować

Asercje

• Dokumentujemy swoje założenia co do wartości zmiennych, na których polegamy za pomocą assert.
• Jeżeli funkcja nigdy nie powinna być wywołana z daną wartością parametru, powinna mieć na początku asercję sprawdzającą czy ten warunek jest spełniony. Do wywołującego ją należy upewnienie się, że podaje jej prawidłowe parametry, a asercja ma za zadanie pomóc w wykryciu przypadków kiedy robi to źle jeszcze podczas developmentu.
• Jeżeli mamy dwa słowniki/zbiory/enumeracje, które powinny z założenia mieć identyczne elementy, należy za definicją dodać sprawdzającą to asercję.
• Jeżeli funkcja zwraca listę, ale wiemy, że w naszym przypadku powinna ona zawierać tylko jeden element, nie ignorujemy pozostałych elementów. Zamiast tego dodajemy asercję sprawdzającą, że lista ma dokładnie jeden element.

Django

Ustawienia

• Każdy setting niepochodzący z samego Django musi być zdefiniowany w pliku z bazowymi settingsami z opisem w komentarzu. Jeżeli nie ma sensownej domyślnej wartości powinien być zakomentowany.
• Nie importujemy settingsów bezpośrednio.
  • 🗸 from django.conf import settings
  • 𐄂 from my_application import settings
• settings/testing.py to settingsy używane jedynie podczas automatycznych testów. Powinny zawierać jak najmniej zmian w stostunku do settings/base.py.
  • W szczególności nie powinny zawierać URLi prawdziwych usług zewnętrznych.
  • Ustawienia, które muszą być zdefiniowane, bo inaczej musielibyśmy dodawać je za pomocą override_settings do każdego testu powinny mieć neutralną domyślną wartość. Najlepiej None lub pusty string.

URLe

• Nie hard-kodujemy URLi. Zawsze używamy funkcji reverse() lub reverse_lazy().

Migracje

• Jeżeli modyfikowana jest postać danych w modelach w sposób niekompatybilny wstecz, kod musi zawierać migrację danych.
• Nie dodajemy nowych zmian do migracji, które zostały już zmerge'owane do mastera.
• Migracje wygenerowane automatycznie przez Django powinny być commitowane bez zmian, ze względu na to, że robiąc rebase często trzeba je wygenerować od nowa.
  • Jeżeli potrzebujemy zmodyfikować migrację, robimy to w osobnym commicie. Dotyczy to w szczególności migracji danych.
• Nie używamy manage.py makemigrations --merge. W razie potrzeby generujemy całą migrację od nowa.

Modele

• Zapisując model do bazy należy zawsze ręcznie odpalić walidacje (metoda full_check()).
• Definiując ForeignKey, OneToOneField, itp. w parametrze podajemy klasę a nie string z jej nazwą, chyba że się nie da (bo tworzy to cykliczną zależność).

Zależności

• Jeżeli kod używa zewnętrznej biblioteki, powinna być ona wymieniona jako zależność w plikach requirements lub w setup.py.
• Każda zależność powinna być dodana zarówno do requirements.txt jak i requirements.lock.
  • requirements.txt zawiera tylko bezpośrednie zależności, analogicznie do pliku Gemfile w Ruby on Rails lub package.json w node.js. Zakres wersji powinien być określony tylko jeżeli wiadomo, że aplikacja nie działa z wersjami biblioteki spoza tego zakresu.
  • requirements.lock zawiera całe drzewo zależności (czyli także zależności naszych zależności) i dla każdej określa dokładną wersję, analogicznie do pliku Gemfile.lock w Ruby on Rails lub npm-shrinkwrap.json w node.js. Plik ten generujemy automatycznie za pomocą pip freeze.

Obsługa błędów

• Kod nie powinien bez ważnej przyczyny wyłapywać wyjątków za pomocą handlera dla Exception lub nawet BaseException (co jest równoważne instrukcji except: bez sprecyzowanego typu).
  • Jeżeli istnieje dobry powód by to robić, informacja o wyjątku powinna być przynamniej zalogowana, razem ze stack trace.
• Nie ignorujemy błędów zwróconych przez funkcje. Jeżeli jest taka konieczność, przynajmniej zapisujemy informacje o błędzie do logu.
• Do logowania używamy modulu logging, a nie funkcji print().

Obsługa błędów (HTTP)

• Zawsze należy sprawdzać status zapytania HTTP.
• Odpowiedź powinna być dekodowana zgodnie z nagłówkiem Content-Type zwróconym przez serwer. Nie zakładamy, że odpowiedź jest zawsze jednego typu (np. zawsze JSON).
  • Jeżeli odpowiedź nie może być zdekodowana zgodnie z zadeklarowanym typem, zwracamy błąd.
• Odpowiedź z API powinna być łatwa do automatycznego przetworzenia przez klienta. Nie powinien być to HTML ani czysty tekst. Najlepiej format taki jak JSON lub XML (wybór formatu zależy od konkretnego API).
  • Niezależnie od formatu ważne jest aby nagłówek Content-Type odpowiedzi pokrywał się z jej zawartością.
• Odpowiedzi zawierające błędy oprócz komunikatu dla człowieka powinny zawierać również kod błędu umożliwiający łatwe odróżenienie różnych sytuacji aplikacji klienta bez polegania na komunikatach, które mogą się zmienić albo zostać przetłumaczone na inny język.

Testy

• Jeżeli naprawiamy błąd, powinien zostać dodany test, który ten błąd ujawnia. Test powinien padać przed zastosowaniem poprawki i przechodzić po naprawieniu kodu.
• Nie testujemy tekstowych komunikatów dla użytkownika, chyba, że są zdefiniowane w globalnych stałych w taki sposób, że testy nie będą się psuły kiedy ktoś tylko przeformułuje ich treść.
• Test składa się z trzech cześci:
  • przygotowanie: tu przygotowujemy dane do testu. Swoje założenia co do danych sprawdzamy za pomocą instrukcji assert (a nie funkcji takich jak assertEqual()). Ewentualne błędy na tym etapie są błędami w założeniach samego testu a nie w testowanym kodzie. Należy zawsze sprawdzać w ten sposób dane, które pochodzą spoza ciała samej funkcji testującej (czyli z fikstur lub z wywołań innych funkcji) i mogą ulec zmianie jeżeli zmieni się kod poza nią.
  • test: tutaj odpalamy test na przygotowanych danych.
  • weryfikacja: tutaj sprawdzamy dane zwrócone przez testowany kod za pomocja funkcji assertEqual() i jej pochodnych.
• Częstym błędem jest pozostawienie niezmienionej nazwy klasy test case'u po utworzeniu nowego zestawu testów na podstawie starego.
• Każdy test powinien definiować wartości ustawień Django, na których polega za pomocą dekoratora override_settings.
• Jeżeli jest wiele różnych poprawnych i niepoprawnych wartości do sprawdzenia, zamiast robić osobny test na każdą lepiej zrobić jeden, który sprawdza je wszystkie w pętli.
• W testach widoków nie trzeba ręcznie tworzyć instancji klasy django.test.Client(). TestCase zawsze dostarcza self.client.

Inne

• Unikamy niepotrzebnych konwersji do int, str, itp. Czasem konwersja jest potrzebna, ale nie powinna być obejściem problemu z danymi, którego nie rozumiemy. Jeżeli oczekujemy liczby a dostajemy string powinniśmy najpierw się zastanowić dlaczego i naprawić przyczynę.
• Nie uciszamy warningów lintera, chyba że wiemy na pewno, że to false-positive. Nawet wtedy staramy się najpierw znaleźć inne rozwiązanie - może nowsza wersja lintera już tego warninga nie generuje, może jest spowodowany jakimś niezwiązanym problemem w naszym kodzie, itp. Jeżeli już musimy warning wyłączyć, robimy to dla pojedynczej linijki.
• Nieużywane parametry funkcji powinny mieć nazwy zaczynające się od podkreślenia (_), co sygnalizuje linterowi, że powinien je zignorować.
  • Pamiętamy o tym, aby usunąć podkreślenie gdy parametr zacznie być używany.
• Użycie funkcji eval() bardzo rzadko jest dobrym pomysłem.
• Enumeracje dziedziczące z klasy Enum powinny mieć dekorator @unique jeżeli wartości kluczy nie mogą się powtarzać.