modex-flexmex / oemof-flexmex

oemof-flexmex is an oemof model built for model comparison within the Modex project FlexMex.
https://oemof-flexmex.readthedocs.io
MIT License
0 stars 0 forks source link

Reuse preprocessing functions #20

Closed unndreay closed 4 years ago

unndreay commented 4 years ago

Implementing issue https://github.com/modex-flexmex/oemo-flex/issues/14

Additionally, a the result comparison similar to https://github.com/modex-flexmex/oemo-flex/issues/13 needs to be fixed before in order to go safe when refactoring.

unndreay commented 4 years ago

https://github.com/modex-flexmex/oemo-flex/pull/20/commits/d878c7f43ab65679c98582dd944eaebf6fa35f91 addresses the issue that we cannot move the functions unless we have made everything a parameter, see https://github.com/modex-flexmex/oemo-flex/issues/14#issuecomment-606065434 (the objected-oriented solution proposed here was postponed)

unndreay commented 4 years ago

Renaming what becomes the preprocessing module for the sake of being in line with the names of the processing steps.

jnnr commented 4 years ago

After talking, we decided to

unndreay commented 4 years ago

For link elements, the merging function remains to be further adjusted.

unndreay commented 4 years ago

Tried to leave changes as minimal as possible for the link_list by doing some replacement only for the new column: https://github.com/modex-flexmex/oemo-flex/blob/c02b645d2fa951a1c821f3233e2cdb2f211cedfb/oemoflex/preprocessing.py#L141-L143 This leaves the postprocessing untouched as far as possible. Changing the link_list itself to the new format (AT_DE) would have entailed much more changes.

unndreay commented 4 years ago

Added suffixes and their processing – probably looks better with DataFrame operations

jnnr commented 4 years ago

I do not understand why travis checks do not appear in this PR. Here I post the results of the last build with some failures for the linters: https://travis-ci.org/github/modex-flexmex/oemo-flex/builds/669838469?utm_medium=notification&utm_source=github_status

jnnr commented 4 years ago

Hey @unndreay, thanks for your effort! This looks good. I left some comments and questions that we should address. After that, this seems ready to merge.