Jump between steps #561

pchklm · 2024-12-11T14:46:37Z

This PR enables the user to skip between steps in order to quickly change parameters and then recalculate the entire workflow.

Changes

Everywhere o7

Testing

Test if steps still get correctly calculated.

PR checklist

Development

If necessary, I have updated the documentation (README, docstrings, etc.)
If necessary, I have created / updated tests.

Mergeability

main-branch has been merged into local branch to resolve conflicts
The tests and linter have passed AFTER local merge

Code review

I have self-reviewed my code.
At least one other developer reviewed and approved the changes

henninggaertner · 2024-12-18T08:30:52Z

You should take a look at #515 if you haven't yet to see whether this should be combined

ronjakrg · 2025-01-10T18:09:57Z

Is this already open for review or still a draft? I tried testing the new icons and apparently the "Calculate"-Button doesn't seem to work and if I open an existing run, all icons are set to the empty circle, although I have already calculated a few steps.

github-actions · 2025-01-23T13:59:12Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
protzilla
disk_operator.py
run.py					150
run_helper.py
steps.py					63, 66, 74, 78-79, 94, 153, 165, 167-168, 486, 651
protzilla/data_analysis
differential_expression_kruskal_wallis.py					227
differential_expression_mann_whitney.py					266
plots.py
protein_graphs.py
ptm_analysis.py					56, 123-125
protzilla/data_integration
database_query.py					113-119
di_plots.py
protzilla/data_preprocessing
outlier_detection.py
plots.py
protzilla/importing
ms_data_import.py					122, 276
protzilla/methods
data_analysis.py					21, 174, 250, 283, 308, 351, 916, 935
data_preprocessing.py
importing.py					144
ui/main
settings.py
ui/runs
fields.py
views.py					21, 254, 288, 497, 701-710
views_helper.py
ui/runs/forms
base.py					101-102, 105-107
custom_fields.py					119
data_analysis.py					323-327, 384, 414, 450, 473, 1188, 1226, 1248
data_integration.py
Project Total

The report is truncated to 25 files out of 60. To see the full report, please visit the workflow summary page.

_{This report was generated by python-coverage-comment-action}

ronjakrg

Sehr schöner und verständlicher Code, und bereichert die Usability von PROTzilla enorm! 🔥 Habe nur kleinere Anmerkungen gemacht, zum Beispiel fehlende Leerzeichen und ein paar überflüssige Print-Statements. :)

protzilla/run.py

protzilla/steps.py

ui/runs/forms/base.py

ui/runs/static/runs/runs.js

ui/runs/views.py

sarahvgls

🐛 BUG 🐛
Ich finde es beeindruckend schade, dass die Tests das nicht abfangen, aber ich kann keinen Run erstellen lol. (Und auch keinen bisherigen testen)
Ich weiß nicht ganz woran es liegt, ob ich vielleicht irgendetwas falsch mache, aber das hier sind meine Beobachtungen: Ich dachte erst es wäre weil die Änderungen nicht backwards-compatible sind - was kein Problem gewesen wäre- aber ich kann leider auch keine Neuen erstellen. Das ist leider für die usability nicht ganz so praktisch haha

Die Landing Page startet und kann wie erwartet verwendet werden. Sobald ich einen alten Run öffne oder einen neuen erstelle können keine Schritte ausgeführt werden. Vielleicht funktioniert der calculate button nicht? Wenn ich clicke, sieht der Button ausgewählt aus (leichter roter Rahmen drum herum), aber es folgt keine Funktion - auch im Backend kommt nichts an:

[2025/02/08 - 14:50:09] [ INFO] Reading yaml from /Users/sarah/Documents/HPI/WS_24_25/BP-Renard/project/PROTzilla2/user_data/workflows/standard.yaml
[2025/02/08 - 14:50:09] [ INFO] "POST /runs/create HTTP/1.1" 302 0
[2025/02/08 - 14:50:09] [ INFO] "GET /runs/detail/test_new_skipping HTTP/1.1" 200 122168
[2025/02/08 - 14:50:09] [ INFO] "GET /static/img/incomplete_icon.svg HTTP/1.1" 304 0

Wenn ich reloade folgt jedes Mal das:

[2025/02/08 - 14:51:15] [ INFO] "GET /runs/detail/test_new_skipping HTTP/1.1" 200 122168
[2025/02/08 - 14:51:15] [ INFO] "GET /static/img/incomplete_icon.svg HTTP/1.1" 304 0

Ich kann zwischen den Schritten springen, aber es passiert halt nichts. Wenn ich andere Schritte als den Ersten versuche zu calculaten passiert gar nichts im Backend.

(Tests laufen problemlos durch leider, gepullt habe ich auch, Code ist auf Stand "add test")

sarahvgls · 2025-02-08T15:37:59Z

Habe nach etwas rumprobieren herausgefunden, dass der Unterschied an der Adresse liegt über die man PROTzilla aufruft:
Auf 127.0.01 funktioniert es - auf localhost (meinem Lesezeichen haha) nicht (port ist der gleiche).
Ich glaube das wäre etwas, was man sich noch mal anschauen sollte, da das unerwartetes Verhalten ist. Eigentlich sollten die beiden Adressen genau das gleiche Verhalten widerspiegeln...
(Auf 0.0.0.0, der dritten unserer erlaubten Adressen nach Django Settings (ui/main/settings.py) geht es auch!)

sarahvgls

Abgesehen vom localhost bug finde ich es super! Vielen Dank!
Ein paar kleine Anmerkungen von mir auch noch, aber sonst spitze :))

protzilla/steps.py

protzilla/run_v2.py

tests/protzilla/test_runner.py

sarahvgls · 2025-02-08T16:19:07Z

tests/protzilla/test_run.py

        step = ImputationByMinPerProtein()
        run_imported.step_add(step)
-        run_imported.step_goto(0, "data_preprocessing")
+        run_imported.step_goto(0, "data_preprocessing_wrong")


Warum wurde das geändert? Testet es dann nicht etwas ganz anderes?

Hier wurde getestet, dass wenn man nach vorne springen will, das nicht funktioniert. Aber genau die Funktionalität wurde ja umgeschrieben. Jetzt probiert er halt zu einem Step einer nicht existierenden Section zu springen. Der Test macht also bisschen was anderes aber hatte ihn trotzdem einfach mal drin gelassen weil mehr Tests immer gut :)

Ahh fair! Dann sinnvoll :))

pchklm · 2025-02-11T23:08:19Z

Habe nach etwas rumprobieren herausgefunden, dass der Unterschied an der Adresse liegt über die man PROTzilla aufruft: Auf 127.0.01 funktioniert es - auf localhost (meinem Lesezeichen haha) nicht (port ist der gleiche). Ich glaube das wäre etwas, was man sich noch mal anschauen sollte, da das unerwartetes Verhalten ist. Eigentlich sollten die beiden Adressen genau das gleiche Verhalten widerspiegeln... (Auf 0.0.0.0, der dritten unserer erlaubten Adressen nach Django Settings (ui/main/settings.py) geht es auch!)

Hm, habs bei mir gerade mal getestet und bei mir funktioniert auch über localhost alles. Idk ob es da noch UNterschiede je nach Betriebssystem oder so gibt.

sarahvgls · 2025-02-13T20:43:53Z

Ich hab noch eine inhaltliche Frage: Wenn ich zb bei unserem Standard Workflow die steps in einer chaotischen Reihenfolge calculate (erst zweites Filter, dann imputation und dann erstes Filtern) Was passiert dann wenn mit dem nächsten Schritt der ausgeführt wird: Nimmt dieser den gefilterten und imputeten Inhalt von allen drei schritten (also werden die noch mal durchgecalculated) oder nur von zweitem Filter + imputation?
Ersteres fände ich sinnvoll, habe mich aber gerade beim ausprobieren gefragt, ob das nicht vielleicht anders ist und dann zu Fehlern führen könnte

sarahvgls · 2025-02-13T20:47:32Z

Übrigens: es geht plötzlich auf localhost. Ich habe leider keine Ahnung warum - ich hoffe mal, ich hatte irgendwas falsch gemacht haha

pchklm and others added 4 commits November 27, 2024 14:11

auto save forms on edit and display correct calculation status per step

028ec7b

final icons, auto calculate/outdate corresponding steps

79a2320

Merge branch 'dev' into enable-skipping-steps

58afa0f

fix merge bugs

741e589

pchklm added 5 commits January 20, 2025 17:08

add failed option, set status to outdated if previous step is deleted

abe3a6c

Merge branch 'dev' into enable-skipping-steps

7ea005f

update step_goto test

b6c9997

fix remaining tests

737db2c

fix even more tests

fec1575

add test

4cc8179

ronjakrg requested changes Feb 3, 2025

View reviewed changes

sarahvgls requested changes Feb 8, 2025

View reviewed changes

sarahvgls reviewed Feb 8, 2025

View reviewed changes

small style fixes etc

9b903fd

pchklm requested review from ronjakrg and sarahvgls February 11, 2025 23:08

ronjakrg approved these changes Feb 12, 2025

View reviewed changes

sarahvgls approved these changes Feb 13, 2025

View reviewed changes

pchklm merged commit 02a7f08 into dev Feb 17, 2025
1 check passed

Jump between steps #561

Jump between steps #561

Uh oh!

Conversation

pchklm commented Dec 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Testing

PR checklist

Uh oh!

henninggaertner commented Dec 18, 2024

Uh oh!

ronjakrg commented Jan 10, 2025

Uh oh!

github-actions bot commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage report

Uh oh!

ronjakrg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sarahvgls left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarahvgls commented Feb 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sarahvgls left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sarahvgls Feb 8, 2025

Choose a reason for hiding this comment

Uh oh!

pchklm Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

sarahvgls Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

pchklm commented Feb 11, 2025

Uh oh!

sarahvgls commented Feb 13, 2025

Uh oh!

sarahvgls commented Feb 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pchklm commented Dec 11, 2024 •

edited

Loading

github-actions bot commented Jan 23, 2025 •

edited

Loading

sarahvgls left a comment •

edited

Loading

sarahvgls commented Feb 8, 2025 •

edited

Loading