isort #110

Open
zelvuska wants to merge 9 commits from feat/isort into master
Owner
No description provided.
zelvuska changed title from WIP: isort to isort 2026-02-17 20:20:19 +01:00
ledoian left a comment
Owner

Docela hit-or-miss, jak se dá od automatizovaného nástroje čekat… Mohlo to být i horší, asi mi je pořadí importů jinak dostatečně jedno, takže nevím, jak by za mě vypadalo ostré zlepšení 🤷

Docela hit-or-miss, jak se dá od automatizovaného nástroje čekat… Mohlo to být i horší, asi mi je pořadí importů jinak dostatečně jedno, takže nevím, jak by za mě vypadalo ostré zlepšení 🤷
aesop/views.py Outdated
@ -13,0 +11,4 @@
from vysledkovky import utils
from .utils import default_ovvpfile
Owner

Je nějaký partikulární důvod, proč to tady vyrobilo dva prázdné řádky? A proč je to nekonzistentní, když třeba v api/urls.py je za impory prázdný řádek jen jeden?

Je nějaký partikulární důvod, proč to tady vyrobilo dva prázdné řádky? A proč je to nekonzistentní, když třeba v `api/urls.py` je za impory prázdný řádek jen jeden?
Author
Owner

Tipuji, že před třídou a před funkcí jsou dva, před proměnnou jeden. Stejně jako v jiných formátítkách Pythonu…

Tipuji, že před třídou a před funkcí jsou dva, před proměnnou jeden. Stejně jako v jiných formátítkách Pythonu…
Owner

Ah, další vynález PEP-8… A máme někdo pocit, že to pomáhá? Já totiž ne – jen to přidává scrollování / ubírá kontext, který vidím na obrazovce; blok importů (zarovnaný v prvním sloupci) od funkce/třídy (odsazený nejmíň jedním tabem) poznám vizuálně rovnou…

Ah, další vynález PEP-8… A máme někdo pocit, že to pomáhá? Já totiž ne – jen to přidává scrollování / ubírá kontext, který vidím na obrazovce; blok importů (zarovnaný v prvním sloupci) od funkce/třídy (odsazený nejmíň jedním tabem) poznám vizuálně rovnou…
Author
Owner

Jo má.

Jo má.
zelvuska marked this conversation as resolved
@ -2,3 +2,4 @@
#from django.db.models import Q
from imagekit.models import ImageSpecField
from imagekit.processors import ResizeToFit, Transpose
Owner

Nevím, jestli bych ImageKit dalo hned vedle Djanga. Also: ten zakomentovaný import by buď měl být zatříděný, nebo smazaný (asi nutný ruční zásah)

Nevím, jestli bych ImageKit dalo hned vedle Djanga. Also: ten zakomentovaný import by buď měl být zatříděný, nebo smazaný (asi nutný ruční zásah)
Author
Owner
  • Nevím, proč jsem dal imagekit k Djangu…
  • Bohužel na zakomentované importy bude vždy potřeba ruční zásah.
- Nevím, proč jsem dal imagekit k Djangu… - Bohužel na zakomentované importy bude vždy potřeba ruční zásah.
Owner

Tak já neříkám, že ruční zásahy nemáme dělat (navíc jsou supposedly jednorázové) :-) Já říkám, že pokud už se někdo snaží dělat systém v importech, tak že bych ocenilo, aby to bylo plošné…

Tak já neříkám, že ruční zásahy nemáme dělat (navíc jsou supposedly jednorázové) :-) Já říkám, že pokud už se někdo snaží dělat systém v importech, tak že bych ocenilo, aby to bylo plošné…
zelvuska marked this conversation as resolved
galerie/views.py Outdated
@ -9,4 +12,2 @@
from personalni.utils import resitel_uzivatele
from galerie.models import Obrazek, Galerie, VZDY, ORG, UCASTNIK, NIKDY
from soustredeni.models import Soustredeni
Owner

Skoro mi přijde, že je praktičtější mít všechny modely u sebe. Ale nevím, jak těžké je to zařídit…

Skoro mi přijde, že je praktičtější mít všechny modely u sebe. Ale nevím, jak těžké je to zařídit…
Author
Owner

Nevím, mám pocit, že těžké. A ono možná chceme spíš stejné balíčky u sebe (aby bylo vidět, co všechno z daného balíčku potřebujeme).

Nevím, mám pocit, že těžké. A ono možná chceme spíš stejné balíčky u sebe (aby bylo vidět, co všechno z daného balíčku potřebujeme).
@ -12,2 +11,3 @@
from mamweb.settings_local import * # Import all the settings for local development
# Import common settings
from .settings_common import * # zatim nutne, casem snad vyresime # noqa
Owner

Tak tohle disapprovuju. Ty overridy tam jsou schválně a to # noqa beztak taky!

Je možné, že se to technicky nerozbilo (v případě, že se podruhé importovaný kód neprovádí), ale vůbec mi nepřijde příčetné se na to spoléhat.

Tak tohle disapprovuju. Ty overridy tam jsou schválně a to `# noqa` beztak taky! Je možné, že se to technicky nerozbilo (v případě, že se podruhé importovaný kód neprovádí), ale vůbec mi nepřijde příčetné se na to spoléhat.
Author
Owner

Dík :)

Dík :)
zelvuska marked this conversation as resolved
@ -8,4 +11,2 @@
#
# Import common settings
from .settings_common import *
Owner

Tady se spíš komentář výš měl změnit na docstring a import měl zůstat tu imho.

I pro produkci a test

Tady se spíš komentář výš měl změnit na docstring a import měl zůstat tu imho. I pro produkci a test
zelvuska marked this conversation as resolved
@ -30,3 +31,4 @@
from ipaddress import ip_network
ALLOWED_HOSTS = [str(ip) for ip in ip_network('192.168.0.0/16')]
Owner

Nepřijde mi zas tak moc užitečné tady rozbíjet tu těsnou vazbu mezi importem a jediným použitím… A když už, tak ten import ip_network může klidně být úplně nahoře (je mi jasné, že tohle se staticky rozhodnout nejspíš nedá)

Nepřijde mi zas tak moc užitečné tady rozbíjet tu těsnou vazbu mezi importem a jediným použitím… A když už, tak ten import `ip_network` může klidně být úplně nahoře (je mi jasné, že tohle se staticky rozhodnout nejspíš nedá)
zelvuska marked this conversation as resolved
mamweb/urls.py Outdated
@ -7,4 +4,1 @@
from django.conf import settings
from django.views.generic.base import TemplateView
from django import views
from django.urls import path # As per docs.
Owner

Není mi moc jasné, co mělo být podle dokumentace, asi možno zrušit?

Není mi moc jasné, co mělo být podle dokumentace, asi možno zrušit?
zelvuska marked this conversation as resolved
mamweb/urls.py Outdated
@ -11,0 +6,4 @@
from django.contrib import admin
from django.contrib.staticfiles.urls import staticfiles_urlpatterns
from django.urls import path # As per docs.
from django.urls import include
Owner

Má někdo vysvětlení, proč zrovna tady je path a include v tomto pořadí?

Má někdo vysvětlení, proč zrovna tady je `path` a `include` v tomto pořadí?
Author
Owner

To by mě také zajímalo :D

To by mě také zajímalo :D
zelvuska marked this conversation as resolved
@ -1,5 +1,7 @@
from django.contrib import admin
from django_reverse_admin import ReverseModelAdmin
from django.contrib import admin
Owner

Huh? Zatímco django_ckeditor_5.widgets to v novinky/admin.py klidně připlácne ke zbytku Djanga, ač je to jiná věc, tak tady to vyrobilo prázdný řádek? Proč?

Huh? Zatímco `django_ckeditor_5.widgets` to v `novinky/admin.py` klidně připlácne ke zbytku Djanga, ač je to jiná věc, tak tady to vyrobilo prázdný řádek? Proč?
Author
Owner

Je třeba přidat django_reverse_admin do skupiny dalšího Djanga v pyproject.toml

Je třeba přidat django_reverse_admin do skupiny dalšího Djanga v `pyproject.toml`…
zelvuska marked this conversation as resolved
@ -13,0 +12,4 @@
vzorecek_na_prepocet)
from personalni.models import Resitel
from tvorba.models import (Cislo, Deadline, Problem, Uloha,
aux_generate_filename)
Owner

Na první čtení mám pocit, že tohle rozlámání je vyloženě rušivé a znepřehledňující. (Neříkám, že si na to nejde zvyknout, jen že to nelahodí mému oku teď…)

Na první čtení mám pocit, že tohle rozlámání je vyloženě rušivé a znepřehledňující. (Neříkám, že si na to nejde zvyknout, jen že to nelahodí mému oku teď…)
Author
Owner

Radši bych měl

odevzdavatko.utils import (
    inverze_vzorecku_na_prepocet,
    vzorecek_na_prepocet,
)

Ale přežiju obojí…

Radši bych měl ``` odevzdavatko.utils import ( inverze_vzorecku_na_prepocet, vzorecek_na_prepocet, ) ``` Ale přežiju obojí…
Owner

Mít

from module import (
<tab>bazmek1,
<tab>bazmek2,
)

mi přijde OK, pokud to tak bude systematicky (tj. asi kdekoliv, kde se to „nevejde“ na jeden řádek).

Mít ``` from module import ( <tab>bazmek1, <tab>bazmek2, ) ``` mi přijde OK, pokud to tak bude systematicky (tj. asi kdekoliv, kde se to „nevejde“ na jeden řádek).
zelvuska marked this conversation as resolved
@ -6,4 +9,2 @@
# Jen typová anotace
from tvorba.models import Problem
from personalni.models import Osoba, Organizator, Resitel, Prijemce
from django.contrib.auth.models import AnonymousUser, User
Owner

Děkujeme, klíčenku neposíláme. Ten komentář tam nebyl zbytečně…

Děkujeme, klíčenku neposíláme. Ten komentář tam nebyl zbytečně…
Author
Owner

A nechceme tam tedy přidat if TYPE_CHECKING, když je důležité, že je to jen typová anotace (protože jakmile někdo přidá něco, co tyhle věci použije na ne-typing, tak komentář zastará aniž by si toho někdo všiml)?

A nechceme tam tedy přidat `if TYPE_CHECKING`, když je důležité, že je to jen typová anotace (protože jakmile někdo přidá něco, co tyhle věci použije na ne-typing, tak komentář zastará aniž by si toho někdo všiml)?
Owner

No, je to nesystematicky jinak řešený type checking proti všemu ostatnímu (co když by ten komentář byl # používáme na zpracování obrázků, třeba někde jinde…), ale takový Python holt trochu je. Asi za mě klidně.

No, je to nesystematicky jinak řešený type checking proti všemu ostatnímu (co když by ten komentář byl `# používáme na zpracování obrázků`, třeba někde jinde…), ale takový Python holt trochu je. Asi za mě klidně.
zelvuska marked this conversation as resolved
@ -4,2 +3,3 @@
from django.contrib.auth import get_user_model
from django.contrib.auth.decorators import permission_required, user_passes_test
from django.contrib.auth.decorators import (permission_required,
user_passes_test)
Owner

Já bych skoro preferovalo dlouhý řádek než náhodné zalomení. A taky mi přijde, že odsazovat mezerami podle závorky je spíš špatně – odsazujeme taby a pokračování dlouhých řádků (třeba v modelech) bývá o tab dál nezávisle na závorkách.

Námitky k lámání a závorkování platí i jinde, nepřišlo mi užitečné to psát ke všem, pokud bude zájem, tak klidně projdu a pohejtím označím

Já bych skoro preferovalo dlouhý řádek než náhodné zalomení. A taky mi přijde, že odsazovat mezerami podle závorky je spíš špatně – odsazujeme taby a pokračování dlouhých řádků (třeba v modelech) bývá o tab dál nezávisle na závorkách. Námitky k lámání a závorkování platí i jinde, nepřišlo mi užitečné to psát ke všem, pokud bude zájem, tak klidně projdu a ~~pohejtím~~ označím
zelvuska marked this conversation as resolved
@ -48,6 +47,7 @@ def sync_skoly(base_url):
from django.urls import reverse
full_url = base_url.rstrip('/') + reverse('export_skoly')
import requests
Owner

Huh, za importem prostě musí být prázdný řádek… Ale ne za from něco import něco o dva řádky výš.

A jo, uznávám, že ten kód je hnusný, ale mám silné tušení, že tohle ho nezlepšuje…

(Chápu to správně, že isort je prostě hloupý, sémantiku Pythonu spíš nechápe a řeší to všechno syntakticky? ☹️)

Huh, za `import`em prostě musí být prázdný řádek… Ale ne za `from něco import něco` o dva řádky výš. A jo, uznávám, že ten kód je hnusný, ale mám silné tušení, že tohle ho nezlepšuje… (Chápu to správně, že `isort` je prostě hloupý, sémantiku Pythonu spíš nechápe a řeší to všechno syntakticky? ☹️)
zelvuska marked this conversation as resolved
pyproject.toml Outdated
@ -87,0 +92,4 @@
default_section = "THIRDPARTY"
known_first_party = ["mamweb"]
known_django = ["django"]
known_takydjango = ["django_countries", "reversion", "rest_framework", "polymorphic", "taggit", "imagekit", "dal", "ckeditor_uploader", "django_ckeditor_5"]
Owner

Už si nepamatuju kde, ale někde máme jiné djangoviny mimo tenhle výčet…

Už si nepamatuju kde, ale někde máme jiné djangoviny mimo tenhle výčet…
@ -5,4 +8,0 @@
from polymorphic.admin import PolymorphicParentModelAdmin, PolymorphicChildModelAdmin, PolymorphicChildModelFilter
from .models import TreeNode, RocnikNode, CisloNode, MezicisloNode, TemaVCisleNode, UlohaZadaniNode, PohadkaNode, UlohaVzorakNode, TextNode, CastNode, OrgTextNode, ReseniNode
from .models import Text, Obrazek
Owner

Trochu váhám, jestli Text a Obrazek nejsou dostatečně odlišné objekty na smíchání se zbytkem, ale možná nejsou (resp. importy se stejně nepíšou na to, aby se nějak moc četly).

Trochu váhám, jestli `Text` a `Obrazek` nejsou dostatečně odlišné objekty na smíchání se zbytkem, ale možná nejsou (resp. importy se stejně nepíšou na to, aby se nějak moc četly).
zelvuska marked this conversation as resolved
@ -6,2 +3,2 @@
from unidecode import unidecode # Používám pro získání ID odkazu (ještě je to někde po někom zakomentované)
from unidecode import \
unidecode # Používám pro získání ID odkazu (ještě je to někde po někom zakomentované)
Owner

Komentář nad, import na jeden řádek. (Že tady ale najednou nevadí, že věci nejsou zarovnané mezerami 🤪)

Komentář nad, import na jeden řádek. (Že tady ale najednou nevadí, že věci nejsou zarovnané mezerami 🤪)
zelvuska marked this conversation as resolved
@ -8,1 +9,4 @@
import soustredeni.models
from tvorba.models import (Cislo, Clanek, Deadline, Problem, Rocnik, # tvorba
Tema, Uloha, ZmrazenaVysledkovka)
Owner

Ten komentář je úplně zbytečný, a navíc se přesunul :-D

Ten komentář je úplně zbytečný, a navíc se přesunul :-D
zelvuska marked this conversation as resolved
tvorba/models.py Outdated
@ -7,1 +7,4 @@
from solo.models import SingletonModel
from unidecode import \
unidecode # Používám pro získání ID odkazu (ještě je to někde po někom zakomentované)
Owner

Když už to musí být bez závorek, tak prosím na jeden řádek…

Když už to musí být bez závorek, tak prosím na jeden řádek…
Owner

Also, odsazovat tabem a ne mezerami! (i v treenode/models.py)

Also, odsazovat tabem a ne mezerami! (i v `treenode/models.py`)
zelvuska marked this conversation as resolved
@ -19,4 +19,3 @@
from django.db.models import Count, Q, Sum
from django.http import Http404, HttpResponse
# Zbytek
Owner

Prázdný řádek spíš nechat?

Prázdný řádek spíš nechat?
zelvuska marked this conversation as resolved
@ -15,4 +11,1 @@
from novinky.testutils import gen_novinky
from personalni.testutils import gen_organizatori, gen_osoby, gen_prijemci, gen_resitele, gen_skoly
from soustredeni.testutils import gen_soustredeni, gen_konfery
from tvorba.testutils import gen_cisla, gen_clanek, gen_dlouhe_tema, gen_rocniky, gen_temata, gen_ulohy_do_cisla, gen_ulohy_k_tematum
Owner

tbf tady to taky dávalo víc smysl před sesortěním… Jakože, asi lítají třísky, ale i tak…

tbf tady to taky dávalo víc smysl před sesortěním… Jakože, asi lítají třísky, ale i tak…
zelvuska marked this conversation as resolved
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/isort:feat/isort
git switch feat/isort

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master
git merge --no-ff feat/isort
git switch feat/isort
git rebase master
git switch master
git merge --ff-only feat/isort
git switch feat/isort
git rebase master
git switch master
git merge --no-ff feat/isort
git switch master
git merge --squash feat/isort
git switch master
git merge --ff-only feat/isort
git switch master
git merge feat/isort
git push origin master
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
mam/mamweb!110
No description provided.