search

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

Feature/refactor mapping into adapter #71

Closed henhuy closed 3 months ago

henhuy commented 4 months ago

Closes #70 Depends on #66 #67

Refactored Mapper class directly into Adapter class. All functions stay the same, but most parameters could be removed and instead are read from class attributes via self.

FelixMau commented 4 months ago

I like the approach of integrating the Mapper into the Adapter looking good and feeling smooth, thank you!

I am afraid that the adapters.py is getting to big and the module has now a mixed task, doing mapping and adapters work while also handling Facade Adapter specific tasks. I would like to separate the Facade specific classes maybe to another module?

henhuy commented 4 months ago

I am afraid that the adapters.py is getting to big and the module has now a mixed task, doing mapping and adapters work while also handling Facade Adapter specific tasks. I would like to separate the Facade specific classes maybe to another module?

I thought the same. But then we have to think how to set up lookup parameter FACADE_ADAPTERS. maybe we should do this in another branch.

nailend commented 4 months ago

Why dont you want to leave it seperate classes and just let the facade-adapter inherit from both or let Adapter inherit from Mapper ?

henhuy commented 4 months ago

Why dont you want to leave it seperate classes and just let the facade-adapter inherit from both or let Adapter inherit from Mapper ?

You mean making Mapper like a Mixin class? I think there would be multiple approaches to divide it. Making Adapter class inherit from Mapper class feels not right, though. For now, I just want to to get on and also think that Adapter class isn't too overloaded yet...