-
Notifications
You must be signed in to change notification settings - Fork 1
Refactoring details #570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactoring details #570
Changes from 11 commits
5d3ce27
a520ef5
b8fb75d
2b9d03b
f336351
97e8425
dab49a3
7f0533e
4249176
6dcb58d
b720c67
d7e0f58
cfeb535
f18060a
f0c53c5
a5bd23c
edf8c6d
e3e6427
2d5dc83
7c22be0
2c0d874
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ich vermute die build_pie_bar_plot Funktion wurde sonst nirgends aufgerufen? Aber ist es vielleicht nicht sinnvoll, wenn man eine generische allgemeine Funktion für diese Art von Plot hat? Für den Fall, dass man sie nochmal verwenden möchte. Also warum hast du das zusammengeführt?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Grundsätzlich bin ich voll bei dir und fände eine allgemeine Funktion auch sehr sinnvoll. Allerdings ist die Funktion an dieser stelle so spezifisch (Bspw. durch sie Labels ["Proteins kept", "Proteins filtered"]), dass man sie sowieso kein zweites mal verwenden kann. Hintergrund war einfach nur, dass ich die zweite Funktion, die nichts tut, entfernen wollte, weil sie nichts tut. Eigentlich ist die Funktion auch genau so gut an anderer Stelle wieder verwendbar, wie vorher, dadurch, dass ich nur Parameter umbenannt habe. Fändest du es trotzdem besser, wenn ich wieder eine zusätzliche Funktion einfüge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warum werden die Parameter alle so spezifiziert? Damit grenzt man ja die Flexibilität der Funktionen schon ein, weil man jetzt nicht mehr alle Parameter, die der Random Forest Classifier von sklearn bietet, nutzen kann.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich habe eigentlich versucht, dass keine Funktionalität oder Flexibiltät verloren geht, sondern nur die **kwargs ausspezifiziert, wenn es konkrete Parameter sind, die verwendet werden oder entfernt, wenn sie überhaupt nicht genutzt werden. Wenn ich etwas übersehen habe, dann ändere ich das gerne. Kannst du mir dafür eine konkrete stelle nennen, an der Flexibilität verloren geht?
Zum Hintergrund, warum ich das gemacht habe: Ich fand es sehr unverständlich, was für parameter bei den Kwargs verwendet erwartet werden und missverständlich, wenn die Funktion kwargs angenommen hat, obwohl mit diesen nichts getan wird. So wollte ich in den Funktionen klarer machen, welches eigentlich die Parameter sind, die genutzt werden.