Skip to content
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

refactor: merge functionality for results #193

Conversation

jsolaas
Copy link
Contributor

@jsolaas jsolaas commented Sep 21, 2023

Make the merge functionality easier to reuse and extend.

@jsolaas jsolaas requested a review from a team as a code owner September 21, 2023 06:34
@jsolaas jsolaas force-pushed the ECALC-136-refactor-time-series-table-data-frame-functionality-in-core-results branch from 24cf571 to 222f93f Compare September 21, 2023 06:36
class TabularTimeSeries(Protocol):
timesteps: List[datetime]

def copy(self) -> Self:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstring?

return merged_object

@classmethod
def merge(cls, *tabular_time_series_list: TabularTimeSeries):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some tests

"""

@classmethod
def _merge_helper(cls, *objects_with_timeseries: ObjectWithTimeSeries) -> ObjectWithTimeSeries:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add test?

Copy link
Collaborator

@TeeeJay TeeeJay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing docs and tests, otherwise ok :)

@jsolaas jsolaas force-pushed the ECALC-136-refactor-time-series-table-data-frame-functionality-in-core-results branch 3 times, most recently from a1e7740 to c22c718 Compare September 21, 2023 18:00
Make the merge functionality easier to reuse and extend.
@jsolaas jsolaas force-pushed the ECALC-136-refactor-time-series-table-data-frame-functionality-in-core-results branch from c22c718 to da6ae59 Compare September 21, 2023 18:03
@jsolaas jsolaas merged commit db1e9b1 into main Sep 22, 2023
4 checks passed
@jsolaas jsolaas deleted the ECALC-136-refactor-time-series-table-data-frame-functionality-in-core-results branch September 22, 2023 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants