-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add real daily aggregation, fix small numbers #269
Merged
ChrisRBe
merged 11 commits into
ChrisRBe:master
from
AlexanderLill:feature_adddailyaggregation
Jan 17, 2021
Merged
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
8564618
Add daily aggregation (default is transaction)
AlexanderLill 0df4213
Fix issues with very small numbers
AlexanderLill 26fb8e5
Change output from 9 to 8 decimals
AlexanderLill 9592979
Update README: Add daily aggregation
AlexanderLill e758898
Merge remote-tracking branch 'origin/feature_adddailyaggregation' int…
AlexanderLill e70ca04
Fix failing tests in statement parser by correcting aggregation
AlexanderLill 20c4035
Try to fix tests
AlexanderLill f8b76ad
Make the linter happy
AlexanderLill 7813cff
test: Increase coverage for daily aggregation
ChrisRBe 0ca6c4b
Merge branch 'master' into feature_adddailyaggregation
ChrisRBe 1b1f0fc
Merge branch 'master' into feature_adddailyaggregation
ChrisRBe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,26 @@ def config_file(self, value): | |
"""config file property setter""" | ||
self._config_file = value | ||
|
||
def __aggregate_statements_daily(self, formatted_account_entry): | ||
entry_date = formatted_account_entry[PP_FIELDNAMES[0]] | ||
entry_type = formatted_account_entry[PP_FIELDNAMES[3]] | ||
logging.debug("entry type is {}. new entry date is {}".format(entry_type, entry_date)) | ||
if entry_date not in self.aggregation_data: | ||
self.aggregation_data[entry_date] = {} | ||
if entry_type in self.aggregation_data[entry_date]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar blocks of code found in 2 locations. Consider refactoring. |
||
logging.debug("add to existing entry") | ||
self.aggregation_data[entry_date][entry_type][PP_FIELDNAMES[1]] += formatted_account_entry[ | ||
PP_FIELDNAMES[1] | ||
] | ||
else: | ||
self.aggregation_data[entry_date][entry_type] = { | ||
PP_FIELDNAMES[0]: entry_date, | ||
PP_FIELDNAMES[1]: formatted_account_entry[PP_FIELDNAMES[1]], | ||
PP_FIELDNAMES[2]: formatted_account_entry[PP_FIELDNAMES[2]], | ||
PP_FIELDNAMES[3]: entry_type, | ||
PP_FIELDNAMES[4]: "Tageszusammenfassung", | ||
} | ||
|
||
def __aggregate_statements_monthly(self, formatted_account_entry): | ||
entry_date = formatted_account_entry[PP_FIELDNAMES[0]] | ||
last_day = calendar.monthrange(entry_date.year, entry_date.month)[1] | ||
|
@@ -115,39 +135,44 @@ def __parse_service_config(self): | |
config = safe_load(ymlconfig) | ||
self.config = Config(config) | ||
|
||
def __process_statement(self, statement, aggregate="daily"): | ||
def __process_statement(self, statement, aggregate="transaction"): | ||
""" | ||
Processes each statement read from the account statement file. First, format in into the dictionary. | ||
Then check what aggregation should be applied. | ||
|
||
- daily: add directly to the output list. | ||
- transaction: add directly to the output list. | ||
- daily: add it to intermediate aggregation collection. | ||
- monthly: add it to intermediate aggregation collection. | ||
|
||
:param statement: Contains one line from the account statement file | ||
:param aggregate: specify the aggregation format; e.g. daily or monthly. Defaults to daily. | ||
:param aggregate: specify the aggregation format; e.g. daily or monthly. Defaults to transaction. | ||
|
||
:return: | ||
""" | ||
formatted_account_entry = self.__format_statement(statement) | ||
if formatted_account_entry: | ||
if aggregate == "daily": | ||
if aggregate == "transaction": | ||
self.output_list.append(formatted_account_entry) | ||
elif aggregate == "daily": | ||
self.__aggregate_statements_daily(formatted_account_entry) | ||
elif aggregate == "monthly": | ||
self.__aggregate_statements_monthly(formatted_account_entry) | ||
|
||
def parse_account_statement(self, aggregate="daily"): | ||
def parse_account_statement(self, aggregate="transaction"): | ||
""" | ||
read a platform account statement csv file and filter the content according to the given configuration file. | ||
If aggregation is selected the output data will be post processed in the following way: | ||
|
||
- aggregate="daily": return the list of processed statements as is. | ||
- aggregate="transaction": return the list of processed statements as is. | ||
- aggregate="daily": return a list of post-processed statements aggregating on daily basis for each | ||
booking type. | ||
- aggregate="monthly": return a list of post-processed statements aggregating on monthly basis for each | ||
booking type. | ||
|
||
:param aggregate: specifies the aggregation period. defaults to daily. | ||
:return: list of account statement entries ready for use in Portfolio Performance | ||
""" | ||
if aggregate == "daily" or aggregate == "monthly": | ||
if aggregate == "transaction" or aggregate == "daily" or aggregate == "monthly": | ||
logging.info("Aggregating data on a {} basis".format(aggregate)) | ||
else: | ||
logging.error("Aggregating data on a {} basis not supported.".format(aggregate)) | ||
|
@@ -163,6 +188,6 @@ def parse_account_statement(self, aggregate="daily"): | |
for statement in account_statement: | ||
self.__process_statement(aggregate=aggregate, statement=statement) | ||
|
||
if aggregate == "monthly": | ||
if aggregate == "daily" or aggregate == "monthly": | ||
self.__migrate_data_to_output() | ||
return self.output_list |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Schaut gut aus.
Ich denke, dass der parser noch mal refaktoriert werden muss, um die gleichanteile loszuwerden. Eventuell was für später.