teach-logarithm about code-review HOT 10 OPEN

a main-t nem is feltétlenül kell külön függvénybe írni

Ezzel nagyon nem értek egyet, mert akkor global scope-ba megy az egész és sok galibát fog okozni, a belülről látható változók, amik random külső változókkal azonos nevűek.

Stimmel. Engem nem érint a dolog, mert meg szoktam írni a main() függvényeket, de sok helyen láttam az alábbi struktúrát.

def fun(x):
    print(f'fun: x = {x}')

if __name__ == '__main__':
    x = 5
    fun(x)

Mivel az x = 5 indentálva volt az if alatt, valahogy sosem gondoltam rá globális változóként, pedig szkriptként hívva tényleg az. Ki is próbáltam, ez is működik:

def fun():
    print(f'fun: x = {x}')

if __name__ == '__main__':
    x = 5
    fun()

Szóval @dlazesz -nek igaza van, a main() függvényt jobb nem megspórolni!

from code-review.

Szia Bálint!

Egyelőre nem foglalkoztam vele sokat, de a következőket látom

• Nem fut le (No such file or directory: 'data/2007_noi_00.csv').
• Tele van nem használt importtal (argparse, sys, numpy, és a sima matplotlib, ha jól látom). Ez nem csak szépséghiba, importálásnál minden csomag beolvasódik a memóriába és végre is hajtódik - ilyenkor feleslegesen.
• Ennek a két sornak mit kéne csinálnia (és miért)?
• Ha az újrafelhasználhatóság szempont (legyen az!), akkor a sample() függvénybe nem égetném bele a names listát, hanem paraméterként adnám át, hogy mire szűrjön - már ha jól értem, hogy mi történik.
• matplotlib-hez sosem volt türelmem, úgyhogy felteszem, hogy a plot() függvény jó.
• Ízlésbeli eltérés: nekem kicsit sok az üres sor. Én alapvetően csak nagyon indokolt esetbe szeretem függvényen belül az üres sorral tagolást, de elfogadom, hogy másnak jobb, ha lazább a kód. Viszont sehogy sem tudok rájönni, hogy itt mi a logikája a tagolásnak. Pl. a 74 és 75. sorok miért tartoznak jobban egymáshoz, mint a 75. és 77. sorok?

A README.md-ben szívesen olvasnék bővebb szöveges kifejtést is!
Szép munka és jó téma!

from code-review.

Az adatok nem közzétehetők, viszont feltettem őket a sisyphos-ra. Ott kiklónozva le kéne futnia.

1. Ha egy nyilvános repó nem használható csak úgy, akkor azt szerintem illik jelezni a README-ben, vagy valami játékadatot kéne tenni a repóba.
2. Tényleg nem tehető közzé, hogy melyik évben melyik nevet hány gyerek kapta? Miért? És ez itt (Lakossági számadatok / Utónév statisztika) mi?

from code-review.

Jogos, javítva. Tudom pycharm. Még sose vettem rá magam... Nincs vmi CLI tool, ami ilyeneket megcsinál, mint a pycharm?

Nem a pycharm az egyetlen, ami tudja ezt, de biztos van CLI is, csak utána kell nézni...

Kiírja a sample() révén kiválogatott adatot, amiből végül a grafikon lesz.

De minek írja ki? Kell valamire? A grafikon ha jól értem közvetlenül a df-ekből jön létre?

Így osztottam be: 68-69. sima grafikon + 71-75. log grafikon + 77-78. jelmagyarázat.

OK, értem!

Plusz dolog: kitaláltam ezt a # ! jelölést arra, hogy ott vmi érdekes / ma-is-tanultam-valamit dolog van. Így grep "# !" *.py után rögtön látom az izgalmas részeket. Van erre esetleg bevett jelölés?

Nem tudom, hogy van-e erre bevett jelölés, a pep8 semmit nem mond róla. Nekem mindenesetre ez nagyon a shebang-re hasonlít (nem pro vagy kontra mondom, csak ez van).

Plusz egy dolog: Ha még csinosítani akarod, akkor a doc string-ekkel lenne érdemes foglalkozni szerintem! Ebben a formában (plot - plot, load - load, sample - sample) nem túl sokatmondóak.

from code-review.

Kiírja a sample() révén kiválogatott adatot, amiből végül a grafikon lesz.

De minek írja ki? Kell valamire? A grafikon ha jól értem közvetlenül a df-ekből jön létre?

Hát nem annyira lényeg, a kiinduló giga adathalmaztól a sample() kiválaszt egy kis részt, amit ábrázol, gondoltam odateszem mellé egy fájlba a konkrét adatokat, amiket a grafikonon látunk.

Plusz egy dolog: Ha még csinosítani akarod, akkor a doc string-ekkel lenne érdemes foglalkozni szerintem! Ebben a formában (plot - plot, load - load, sample - sample) nem túl sokatmondóak.

Jogos, persze. :)
Itt főként a main()-nak a docstringjével szokott gondom lenni.
Általában az van, hogy legszívesebben bemásolnám a főfő docstringet, de azt meg minek. :)

Kösz a megjegyzéseket!

from code-review.

a main-t nem is feltétlenül kell külön függvénybe írni

Ezzel nagyon nem értek egyet, mert akkor global scope-ba megy az egész és sok galibát fog okozni, a belülről látható változók, amik random külső változókkal azonos nevűek.

Javasolnám a main() függvényt, amit az if __name__ == '__main__': meghív és így ettől védve vagy.

Azt hittem, a nytud automatikusan tudja ezeket az adatokat, csak valami rossz beidegződés miatt "titkolja".

Ezeket jó így utólag, véletlenül megtudni. :D

Az van, hogy az első 100 nevet mindig közzéteszik, de az egészet nem. Rejtély, hogy miért.

Tipp: GDPR, személyiségi jogok? A Zipf görbe miatt az egyedi neveknél viszonylag jól ki lehet következtetni, kiről van szó. talán ezért vágnak.

Nekem mindenesetre ez nagyon a shebang-re hasonlít (nem pro vagy kontra mondom, csak ez van).

Nekem is egyből ez jutott eszembe. -> Ezért NEKEM nem tetszik.

Nem a pycharm az egyetlen, ami tudja ezt, de biztos van CLI is, csak utána kell nézni...

Sőt! Túl sok ilyen van és azokat be kell állítgatni, az a baj. A Pycharm, "meg csak működik". A VSCode-ban nézegettem a beköthető programokat, de fél percnél tovább tartott és nem mentem mélyebbre.

Sztem jó. :) Én is most vettem rá magam a matplotlib-re, nekem tetszik.

A seaborn-t szokták még szeretni, mert egyszerűbb/pythonikusabb mint a matplotlib. A matplotlib inkább a MATLAB-ra akar hajazni, amit ha valaki nem ismer vagy nem szeret, annak bonyolult.

A kódról:

SZVSZ a semmitmondó általános függvénynevek a) összeakadnak a különféle library-kban ha refaktorálunk vagy keresünk ezért kerülendők, b) helyett kifejtős függvényneveket használnék, ha nem annyira általános a függvény. A docstring és kommentekkel szemben a hívásnál látszik, hogy mit csinál a függvény és nem kell lemenni a definícióig a megértéshez.

A # !-nél jó lenne szövegesen odaírni, hogy miért is érdekes. Elvégre 3rd party-knak készül a kód, azért nyilvános.

A többit Iván már mondta. Csak így tovább! :)

from code-review.

Mivel az x = 5 indentálva volt az if alatt, valahogy sosem gondoltam rá globális változóként, pedig szkriptként hívva tényleg az. Ki is próbáltam, ez is működik:

A Pycharm ilyenkor aláhúzza a belső példányt (ha jól be van állítva), hogy jelezze, hogy most épp "felüldefiniálsz" egy globálist, amit nem biztos, hogy így tervezel.

Szóval @dlazesz -nek igaza van, a main() függvényt jobb nem megspórolni!

Köszi! Szent igaz! :)

from code-review.

Köszi Iván! :)

Nem fut le (No such file or directory: 'data/2007_noi_00.csv').

Az adatok nem közzétehetők, viszont feltettem őket a sisyphos-ra. Ott kiklónozva le kéne futnia.

Tele van nem használt importtal (argparse, sys, numpy, és a sima matplotlib, ha jól látom). Ez nem csak szépséghiba, importálásnál minden csomag beolvasódik a memóriába és végre is hajtódik - ilyenkor feleslegesen.

Jogos, javítva. Tudom pycharm. Még sose vettem rá magam... Nincs vmi CLI tool, ami ilyeneket megcsinál, mint a pycharm?

Ennek a két sornak mit kéne csinálnia (és miért)?

Kiírja a sample() révén kiválogatott adatot, amiből végül a grafikon lesz.

Ha az újrafelhasználhatóság szempont (legyen az!), akkor a sample() függvénybe nem égetném bele a names listát, hanem paraméterként adnám át, hogy mire szűrjön - már ha jól értem, hogy mi történik.

Jó, jó... :) Egyébként jól érted.

matplotlib-hez sosem volt türelmem, úgyhogy felteszem, hogy a plot() függvény jó.

Sztem jó. :) Én is most vettem rá magam a matplotlib-re, nekem tetszik.

Ízlésbeli eltérés: nekem kicsit sok az üres sor. Én alapvetően csak nagyon indokolt esetbe szeretem függvényen belül az üres sorral tagolást, de elfogadom, hogy másnak jobb, ha lazább a kód. Viszont sehogy sem tudok rájönni, hogy itt mi a logikája a tagolásnak. Pl. a 74 és 75. sorok miért tartoznak jobban egymáshoz, mint a 75. és 77. sorok?

Így osztottam be: 68-69. sima grafikon + 71-75. log grafikon + 77-78. jelmagyarázat.

A README.md-ben szívesen olvasnék bővebb szöveges kifejtést is!

Hm.. gondolkodom rajta.

Szép munka és jó téma!

:)

Plusz dolog: kitaláltam ezt a # ! jelölést arra, hogy ott vmi érdekes / ma-is-tanultam-valamit dolog van. Így grep "# !" *.py után rögtön látom az izgalmas részeket. Van erre esetleg bevett jelölés?

from code-review.

Az adatok nem közzétehetők, viszont feltettem őket a sisyphos-ra. Ott kiklónozva le kéne futnia.

1. Ha egy nyilvános repó nem használható csak úgy, akkor azt szerintem illik jelezni a README-ben, vagy valami játékadatot kéne tenni a repóba.

Igen, igen. :)

2. Tényleg nem tehető közzé, hogy melyik évben melyik nevet hány gyerek kapta? Miért? És ez itt (Lakossági számadatok / Utónév statisztika) mi?

Az van, hogy az első 100 nevet mindig közzéteszik, de az egészet nem. Rejtély, hogy miért. Egyszer megvette tőlük az intézet az akkor aktuális adatokat, és mivel pénzért adták, ezért tippelem, hogy nem örülnének, ha közzétennénk.

from code-review.

Az van, hogy az első 100 nevet mindig közzéteszik, de az egészet nem. Rejtély, hogy miért. Egyszer megvette tőlük az intézet az akkor aktuális adatokat, és mivel pénzért adták, ezért tippelem, hogy nem örülnének, ha közzétennénk.

OK, értem. Azt hittem, a nytud automatikusan tudja ezeket az adatokat, csak valami rossz beidegződés miatt "titkolja".

Itt főként a main()-nak a docstringjével szokott gondom lenni.
Általában az van, hogy legszívesebben bemásolnám a főfő docstringet, de azt meg minek. :)

A main() nem túl fontos szerintem, az csak a parancssori hívhatóságot biztosítja. A lényeg a tényleges függvények dokumentációja (a main-t nem is feltétlenül kell külön függvénybe írni, mehet közvetlenül is az if __name__ == '__main__': alá, úgy is mindenki érteni fogja).

from code-review.