Skip to content

Feature: select+joins analysis with pandas DataFrames#4

Merged
il-ia-ni merged 60 commits intomainfrom
feature-joins-analysis-w-pandas
Feb 24, 2023
Merged

Feature: select+joins analysis with pandas DataFrames#4
il-ia-ni merged 60 commits intomainfrom
feature-joins-analysis-w-pandas

Conversation

@il-ia-ni
Copy link
Copy Markdown
Owner

@il-ia-ni il-ia-ni commented Feb 2, 2023

Hey Phillip :)

This is my first commit to the branch for testing pandas dataframes with the SQL Alchemy lists of select+join-results.

Right now there seem to be problems with filling DataFrames with data, even though the sqlalchemy lists are correctly parsed to DataFrames. This should have something to do with the detached status of the lists upo closing the Session.

Here are the links to the topic:

  1. Error: Parent instance is not bound to a Session; (lazy load/deferred load/refresh/etc.) operation cannot proceed
  2. State Management of SQL Alchemy query results (only for ORM-instances): https://docs.sqlalchemy.org/en/14/orm/session_state_management.html#session-object-states

@il-ia-ni il-ia-ni self-assigned this Feb 2, 2023
@il-ia-ni il-ia-ni marked this pull request as ready for review February 7, 2023 10:01
@il-ia-ni
Copy link
Copy Markdown
Owner Author

il-ia-ni commented Feb 15, 2023

Hier bin ich mir nicht sicher: grafik

Die Funktion ist ja sicherlich zu Testzwecken gedacht. Aber wir müssen nochmal schauen/recherchieren, ob es Standard ist, in einer Funktion drei mal session.execute() aufzurufen. Und da wir kein session.add_all() verwenden, frage ich mich,ob das session.commit() notwendig ist.

Bin mir auch hierzu nicht so ganz sicher! Da SQL Alchemy in der Dokumentation den "Commit as you go" Weg immer nimmt (so schreiben sie es selber), habe ich es auch in meiner Testmethode übernommen. Sonst muss ich noch rausfinden, wie man diesen .begin() auf eine Session-Instanz anwendet...

Hier ist was in der SQL Alchemy Dokumentation zu dem Unterschied zwischen den "Commit as you go"- und "Begin once"-Commitprinzipien steht:

“Begin once” style is often preferred as it is more succinct and indicates the intention of the entire block up front. However, within this tutorial we will normally use “commit as you go” style as it is more flexible for demonstration purposes.

@il-ia-ni
Copy link
Copy Markdown
Owner Author

Hey @Phillip-Oliver , ich habe heute alle Neuerungen hochgeladen!
Unter anderem sind nun alle meinen Methoden für Erstellung von DataFrames in die Package data_analysis migriert worden.

Ich habe auch deine beiden Skripts (defect_event_root_cause und defect_root_cause_pairs) auch im data_analysis Package überarbeitet und ein Paar interessanten Sachen da rausgefunden.

Auch meine Test Datenbank ist nun komplett dem Schema mit dem falschen Namen losgeworden, alle Tabellen, Prozeduren und Funktionen sind nun unter dem Schema "Main" zu finden: Dies ermöglicht eigentlich, diese PR zu schließen und dann kann ich die nächste PR mit der lokalen fallback-SQLite DB fertig machen!

Also bitte nochmals um deine Kommentare :)

Copy link
Copy Markdown
Collaborator

@Phillip-Oliver Phillip-Oliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stabile Arbeit !
Kannst ja mal über die Kommentare rüber gehen. Aber ich denke, wir können es bald mergen ;)

Comment on lines +17 to +28
def create_df_from_list(rows_data_list: list, columns_list: list) -> pd.DataFrame:
"""
Creates a pandas DataFrame based on a list of selection result Rows instances that were already created using f.e.
SQL Alchemy Core API or ORM API
:param rows_data_list: a list of Rows instances of a select result
:param columns_list: an iterable with the strings of attributes names of the select result
:return: a pandas dataframe with the data of the list with the selection result
"""
result_df = pd.DataFrame(rows_data_list, columns=columns_list)
logger.debug("A DataFrame with following parameters was created from the list: \n", result_df.info())

return result_df
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Für die Umwandlung einer Liste in einen DF extra eine Funktion ? ;)

Copy link
Copy Markdown
Owner Author

@il-ia-ni il-ia-ni Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tja danke für den Hinweis 😆 Mir ist während der Verbesserung des Codes irgendwie komplett entlaufen, dass ich hier was anderes machen wollte... Meine Idee war eigentlich noch eine statische Typ-Prüfung einzuführen. Ansonsten macht die Methode gar keinen Sinn.

Ich habe sie jetzt mit Union Type Annotations erweitert, damit nur Rows von SQL Alchemy akzeptiert werden. Hier ist der Commit dazu. Auch den Methodennamen habe ich angepasst.

Theoretisch könnte man weitere Validierungen von Rows vor der Erstellung von DataFrames einführen. Es war auf jeden Fall eine gute Übung zu den Type Annotations in Python 😉

Comment on lines +14 to +47
create_root_cause_groups_text_clause = text(
"WITH A AS ("
" SELECT A.create_date, A.event_id, B.signal_id, A.behaviour_pattern_id, A.strand_id, A.slab_id, B.importance "
" FROM main.defect_event A INNER JOIN main.defect_root_cause B "
" on A.event_id=B.event_id "
" WHERE A.event_id IN ("
" SELECT event_id "
" FROM main.defect_root_cause "
" WHERE signal_id NOT in :signals "
" AND slab_id IS NOT null "
" AND create_date BETWEEN CAST(:start_date AS DATE) AND GETDATE() "
" GROUP BY event_id HAVING COUNT(event_id) > 1 AND COUNT(event_id) < 3)"
" )"
"SELECT "
" create_date, "
" event_id, "
" behaviour_pattern_id, "
" strand_id, "
" slab_id, "
" importance,"
" STUFF((SELECT ',' + CAST(signal_id AS varchar(1000)) "
" FROM A AS innerTable "
" WHERE innerTable.event_id = p.event_id "
" FOR XML PATH('')),1,1,'') "
"AS signal_pattern "
"FROM A AS p "
"GROUP BY p.event_id, create_date, behaviour_pattern_id, strand_id, slab_id, importance")


def root_cause_pairs_query(date: str, signals: tuple[str, ...]) -> str:
# tuple[str] vs tuple[str, ...] see: https://stackoverflow.com/questions/72001132/python-typing-tuplestr-vs-tuplestr

# f"" is formatted string literal. An overview of Python String formatting: https://realpython.com/python-f-strings/
# f""" """ allows writing a multiline string with no escape character \ before each new line
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Und was von beidem funktioniert besser ? :)

Copy link
Copy Markdown
Owner Author

@il-ia-ni il-ia-ni Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auf jeden Fall sind die f-Strings die beste Lösung! 😄
Das einzige, worauf man aufpassen muss ist dass die Strings manchmal von SQL Alchemy in integers umgewandelt werden, auch wenn es eigentlich ein Datum ist... Das habe ich für die Zeile mit der skalaren Variable "date" wie folgt gelöst:
AND create_date BETWEEN CAST(\'{date}' AS DATE) and GETDATE()

Das ist aber immer noch 1000 Mal einfacher, als für text-Konstrukte die ganzen Binding-Methoden für nicht-skalare Variablen benutzen zu müssen... War aber auf jeden Fall der Übung wert 📌

Comment on lines +85 to +88
query = root_cause_pairs_query(date=date, signals=signals_to_filter)
with session as s:
# result = s.execute(text_statement, params={'signals': signals_to_filter, 'start_date': date})
result = s.execute(query)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wäre es vllt. auch möglich einen Filter für caster einzubauen?
Fällt mir gerade so ein !

Die caster wären '2c' oder '1'

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich habe nun den Filter im extract_root_cause_pairs Skript eingeführt. Hier ist der Commit dazu.

Es gibt jedoch ein ziemlich großes Problem mit den SQLAlchemy Statements in Form der f-Strings: Ein Tuple in Python darf nur dann einen Wert haben, wenn hinter ihm ein Komma steht. Solchen Tuple schafft aber SQL nicht in der "WHERE IN ('1', )"-Anweisung zu erledigen.

Ich habe deswegen TODOs reingeschrieben, da ansonsten man doppelt einen denselben Wert reingeben muss (siehe die Zeile 128) 😱

global_engine = make_engine(url, 1000)
global_session = get_session(global_engine)
debug_format() # adds a custom format for debug-level of loguru (saving local .log-files)
global_session = get_session(sqlserver_engine)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich glaube diese plotting Sachen würde ich in ein extra modul packen, also nicht in main.py.

Hast du das mal mit Testdaten ausprobiert ob das läuft ?

Copy link
Copy Markdown
Owner Author

@il-ia-ni il-ia-ni Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, ich habe nun das Main.py vom Testcode befreit und ein plots.py Skript für die Matplotlib-basierten Methoden erstellt. Die Methode fürs Gruppieren der Signaldaten von Events habe ich ins dataframes.py Skript verschoben. Hier ist der Kommit dazu.

Es sind nun auch DocString zu jeder Methode drin sowie ein Paar To-Dos 😃

Leider kann ich keine Screenshots mehr auf dem Master Client auf GutHub hochladen... 🙄 Nach dem ich die Datanbank restrukturiert habe, habe ich tatsächlich mit ein Paar klinDensätzten signal_data aus der KundenDB bezogen und Plots erstellt! Es klappt alles gut 🏅

@il-ia-ni il-ia-ni merged commit 2d45645 into main Feb 24, 2023
@il-ia-ni il-ia-ni deleted the feature-joins-analysis-w-pandas branch February 24, 2023 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants