[JajkoJajkoJajkoJajko]

#programowanie #naukaprogramowania #python #postgresql #programista15k

tl;dr Proszę o ocenę mojego pierwszego kodowego projektu :)

w ramach nauki programowania tworzę sobie bibliotekę ułatwiającą komunikację pythona, a konkretnie biblioteki pandas z bazą postgresql. Oczywiście cały projekt jest dość w powijakach, a z programowania jestem raczej początkujący, więc pewnie roi się od bugów i brzydkiego kodu. Dlatego chciałbym Was prosić o opinię w każdym aspekcie, zwłaszcza o wskazanie co jest do poprawy zarówno w projekcie, jak i w sposobie mojego kodowania. Poniżej link do GitHuba:

Link

idea opiera się na tym, żeby w móc w prosty sposób wczytywać dane z db prosto do DataFrame, ale też w prosty sposób przeprowadzać inserty, update'y i wyciągać podstawowe informacje nt. tabel, kolumn, kluczy głównych, schematów itp. Nie mogłem za bardzo znaleźć biblioteki która by DataFrame'y łatwo ładowała do bazy, więc stwierdziłem że sam napiszę, ale jeśli ktoś takie zna (oprócz samego pandas, który ma mocno ograniczone funkcjonalności w tym zakresie), to super by było, gdyby się podzielił ;)

wołam także @Kiro i @biwalencik, z którymi niedawno przeprowadziłem krótką dyskusję na temat sensu pisania takiej biblioteki, może będziecie zainteresowani pierwszymi wynikami :)

[jascen]

@JajkoJajkoJajkoJajko: ile czasu zajeło Ci to?

[dog_meat]

@JajkoJajkoJajkoJajko: Nigdzie nie zamykasz połączenia do bazy. Może lepiej byłoby to zrobić jako context manger? W konstruktorze, jeśli nie uda się nawiązać połączenia, powinieneś rzucić wyjątek. W przeciwnym razie program nie wie, że nie udało się nawiązać połączenia i wykrzaczy się na próbie dostępu do self.cursor (który nie będzie zdefiniowany)

[dog_meat]

@JajkoJajkoJajkoJajko: Nie musisz tworzyć atrybutów typu self.db_name itp. i tak z nich nie korzystasz. wystarczy Ci self.connection i self.cursor

[dog_meat]

@JajkoJajkoJajkoJajko: Nigdy, ale to NIGDY nie składaj zapytań SQL poprzez łączenie stringów, tak jak robisz to teraz. To aż się prosi o SQL Injection

[dog_meat]

@JajkoJajkoJajkoJajko Zamiast konstrukcji typu if isinstance(schemaname, str)==False: użyt po prostu if isinstance(schemaname, str):

Ogólnie to ja bym nie sprawdzał tego is instance wcale w tym miejscu

[JajkoJajkoJajkoJajko]

@jascen: około półtora miesiąca, z tym że robiłem to po pracy i w weekendy no i też nie dzień w dzień. także ciężko określić ile godzin :/
@dog_meat: rzeczywiście nie pomyślałem o zamykaniu połączenia. myślisz że przy każdym wywołaniu funkcji powinno się łączyć i po wykonaniu funkcji zamykać? co do self.db_name to korzystam właśnie przy nawiązywaniu połączenia :)

[meohaw]

@JajkoJajkoJajkoJajko: ale zrób pan z tego normalną paczkę taką, żeby chociaż miała normalną strukturę, jakieś requirements.txt, mikro README ze sposobami użycia etc. etc.

Wcięcia to powinny być 4 spacje a nie twardy tabulator.

Pomyśl czy nie warto by napisać troszkę testów jednostkowych, przynajmniej do najważniejszych

[JajkoJajkoJajkoJajko]

Nigdy, ale to NIGDY nie składaj zapytań SQL poprzez łączenie stringów, tak jak robisz to teraz. To aż się prosi o SQL Injection

@dog_meat: tak tak, zdaję sobię sprawę. tam gdzie użytkownik podaje dane, tam jest to odpowiednio zabezpieczone, ale chyba rzeczywiście nie poprawiłem tego w kilku funkcjach, które wczesniej zdefiniowałem. mógłbyś mi tylko jeszcze wytłumaczyć o co chodziło z tym zbędnym self.db_name?

[dog_meat]

@JajkoJajkoJajkoJajko: Jeśli chodzi o self.db_name, to używasz tego tylko w konstruktorze, więc nie musisz z tego tworzyć atrybutu, dla całej, klasy bo i tak w innych metodach tego nie używasz.

Zamykanie połączeń robiłbym przez kontekst managera (chyba najlepiej jako osobną klasę), która zwróci Ci obiekt Twojej klasy do operowania na nim w bloku with.

[meohaw]

@JajkoJajkoJajkoJajko

Tam gdzie robisz p.connect() to sklejasz niezdarnie te stringi, zamiast używać keyword arguments

za http://initd.org/psycopg/docs/module.html

[dog_meat]

@JajkoJajkoJajkoJajko: psycopg (jak chyba każda sensowna biblioteka bazodanowa) obsługuje prepared statements, więc skorzystaj z tego, zamiast łączyć stringi

[JajkoJajkoJajkoJajko]

@meohaw: stworzenie normalnego pakietu będzie kolejnym krokiem, muszę póki co poczytać jak to się robi, póki co skupiałem się na bebechach. Do tego cholernego tabulatora się przyzwyczaiłem, trzeba to zmienić.

dog_meat dobrze mówi, zamykaj połączenie do bazy, ja bym trzymał otwarte tak długo jak żyje ten twój obiekt DBConnection (swoją drogą słaba nazwa na coś co robi tyle rzeczy, a nie tylko nawiązuje połączenie do bazy) a jak umiera to

[JajkoJajkoJajkoJajko]

@meohaw: @dog_meat: super, dzięki za uwagi, zaraz się biorę za wdrażanie. Mam nadzieję, że poza tym wygląda w miarę sensownie :P za jakiś czas wrzucę update :)

[meohaw]

@JajkoJajkoJajkoJajko ja bym tam dalej wczytywał rzeczy do pandas z użyciem pandas.readsql(), tam można normalnie podać nazwę tabeli do wczytania, połączenie do bazy i reszta się robi bardzo automagicznie.

http://pandas.pydata.org/pandas-docs/version/0.20/generated/pandas.read_sql.html

Czego ci tam brakowało tak konkretnie? W sumie tak samo w tosql, czego ci brakowało przy zapisie?

Pandas się lubi z SQLAlchemy, więc one razem całkiem sprawnie robią większość operacji, dlatego nie znam biblioteczki która by robiła to co

[JajkoJajkoJajkoJajko]

@meohaw: zająłem się czymś takim, bo uczę się też korzystania ze scikit-learn w połaczeniu z pandas, i wiem że dobra dwustronna komunikacja z bazą danych będzie mi niezbędna

wczytywanie do pandas to zrobiłem już trochę z rozpędu żeby było, ale czy jest jakaś biblioteka która w miarę łatwo pozwala robić update'y i inserty? nic nie mogłem znaleźć takiego, więc postanowiłem sam zrobić

[JajkoJajkoJajkoJajko]

@meohaw: aa, może nie zrozumiałem. jeśli chodzi o to_sql to próbowałem korzystać ale jakoś nie potrafiła ta funkcja aktualizować wierszy w razie duplikatów. głównie to mnie skłoniło do napisania czegoś swojego, bo bez sensu wydaje mi się usuwanie tabeli i wrzucanie na nowo za każdym razem, zwłaszcza jak ma kilkaset tysięcy rekordów, albo i więcej

[biwalencik]

@JajkoJajkoJajkoJajko: Chyba wczesnej sugerowalem przejżenie biblioteki odo. Cudo z ekosystemu Blaze, ktore przerzuci dowolne dane w dowolne dane. Moze troche przesadzilem, ale z pewnoscia da radę wpakować DF do postgresa.

http://odo.pydata.org/en/latest/overview.html

Nie ma co wynajdywac kola od nowa.

odo(df, 'postgresql://hostname::tablename')

[JajkoJajkoJajkoJajko]

@biwalencik: o, wiesz może, czy potrafi odo aktualizować tylko niektóre wiersze w tabeli? tzn jeśli np tabela ma kolumny (id, imie, nazwisko) gdzie id to klucz główny, to jeśli zapodasz jej (id, nazwisko) to zmieni nazwisko w odpowiednim rekordzie po kluczu głównym?

Jeśli tak to może rzeczywiście to wszystko niepotrzebne :P ale co się nauczyłem to moje :D

[biwalencik]

@JajkoJajkoJajkoJajko:

Potrafi wszystko to co sqlAlchemy i tam też wcześniej Cie kierowałem.

To również jeden z głównych mankamentów Twojego kodu. Używasz formatowania stringów do generowania zapytań SQLa i wrzucasz je na exec do

[JajkoJajkoJajkoJajko]

@biwalencik: hmm. to w takim razie będę musiał posiedzieć dłużej nad dokumentacją sqlAlchemy. Zdaję sobie sprawę, że część moich rozwiązań jest dość naiwna, ale wynika to głównie z tego że mało jeszcze umiem, ale też z tego że szyłem to na swoje potrzeby, a nigdzie takie rozwiązania nie mogłem znaleźć. Jeśli znajdę to, co mnie interesuje w odo lub sqlAlchemy to z pewnością to wdrożę