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.

Sign up for GitHub

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

chore: remove unused methods #704

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: remove unused methods #704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/ecalc_cli/io/output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,12 @@ def export_tsv(
prognosis_filter = config.filter(frequency=frequency)
result = prognosis_filter.filter(ExportableGraphResult(results), resampled_periods)

row_based_data: dict[str, list[str]] = CSVFormatter(
separation_character="\t", index_formatters=PeriodFormatterConfig.get_row_index_formatters()
).format_groups(result)
row_based_data: dict[str, list[str]] = {
key: CSVFormatter(
separation_character="\t", index_formatters=PeriodFormatterConfig.get_row_index_formatters()
).format(val)
for key, val in result.items()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for key, val in result.items()
for key, val in result.groups

Maybe we should move the loop in groups in here instead, since FilteredResult don't need to know about Formattable. Unnecessary coupling

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm struggling with the directory structure we currently have. Should Formattable be placed in domain, FormattableGroupedQuery in infrastructure etc.? Or should we have a similar structure within presentation.exporter? What is "important" enough to be considered domain. When I think about it I just want to separate interfaces from implementations, so formattable could be in the domain, but it isn't what I think of as the core ecalc domain.....Should Formatter be a completely separate module?

}

exporter = Exporter()
exporter.add_handler(
Expand Down
11 changes: 1 addition & 10 deletions src/libecalc/presentation/exporter/formatters/formatter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import abc

from libecalc.presentation.exporter.formatters.formattable import Formattable, FormattableGroup
from libecalc.presentation.exporter.formatters.formattable import Formattable
from libecalc.presentation.exporter.formatters.index_formatter import IndexFormatter


Expand All @@ -13,9 +13,6 @@ def format(self, table: Formattable) -> list[str]:
"""
...

@abc.abstractmethod
def format_group(self, groups: FormattableGroup) -> dict[str, list[str]]: ...


class CSVFormatter:
def __init__(self, separation_character: str = ",", index_formatters: list[IndexFormatter] = None):
Expand All @@ -42,9 +39,3 @@ def format(self, tabular: Formattable) -> list[str]:
row = [str(tabular.get_value(row_id, column_id)) for column_id in column_ids]
rows.append(self.separation_character.join([*row_index, *row]))
return rows

def format_groups(self, grouped_tabular: FormattableGroup) -> dict[str, list[str]]:
csv_formatted_lists_per_installation: dict[str, list[str]] = {}
for group_name, tabular in grouped_tabular.groups:
csv_formatted_lists_per_installation[group_name] = self.format(tabular)
return csv_formatted_lists_per_installation
Loading