sedos-project / data_adapter_oemof

This respository holds the data adapters to connect oemof with the OEDatamodel-concrete.
GNU Affero General Public License v3.0
0 stars 1 forks source link

Special parameter mapper and Test Goal improvements #54

Closed FelixMau closed 7 months ago

FelixMau commented 8 months ago

fixes #52 fixes #51

FelixMau commented 8 months ago
henhuy commented 8 months ago

Looks good on first sight. As Julian suggested, I would fix build_datapackage test, aka check if test data fits goal data. For this we should adapt test, in a way that column order does not have to be the same, only data. I think this could be done by loading result CSVs into dataframe and compare them like in this link: https://stackoverflow.com/a/21000675/5804947

FelixMau commented 8 months ago

Sorry I keep things even more seperate in future.


I am a bit lost, contentwise, as https://github.com/sedos-project/data_adapter_oemof/issues/52 doesn't give much context. Probably @henhuy has a better understanding of what's done here, but I need more context for a proper review.

Yes sorry I derived the task from bilateral chat with Hendrik in do_not_init_facade #47


The test is also still skipped and black wants a small change. Besides this, the changes seem to be fine, from what I can tell.

Yes I cannot really run the test_build_tabular_datapackage_from_adapteruntil Hendrik or me repairs the faulty timeseries in adapter. But I can work out the Goal data by hand a little so we get something to work with and maybe "handwritten" goal data will be better anyways (of course in a new PR)

EDIT I made a mistake and was using the wrong environment. Now I will check on this test as well of course.


As Julian suggested, I would fix build_datapackage test, aka check if test data fits goal data. For this we should adapt test, in a way that column order does not have to be the same, only data. I think this could be done by loading result CSVs into dataframe and compare them like in this link: https://stackoverflow.com/a/21000675/5804947

The Goal data is cheked (for test_build_datapackage) but I will improve the assertion for sure.


Edit: The check_if_csv_dirs_equal actually already uses pandas and check_like is already set to True to not check order of Index & Columns

FelixMau commented 7 months ago

I would like to merge here and make a seperate PR to fix the hack-a-thon Datapackage since it requires some more changes that would make this PR a little confusing.