Skip to content

Update to reflect latest changes in Revolut outputs#30

Open
MarvinRucinski wants to merge 3 commits intopbialon:mainfrom
MarvinRucinski:main
Open

Update to reflect latest changes in Revolut outputs#30
MarvinRucinski wants to merge 3 commits intopbialon:mainfrom
MarvinRucinski:main

Conversation

@MarvinRucinski
Copy link
Copy Markdown
Contributor

Improvements

  • Improved stock operation parsing - the Total Amount row now includes the currency
  • Added support for more operation types - previously, the "BUY - STOP" operation was unhandled; now any operation containing "BUY" is treated as a BUY operation
  • Improved logging
  • Improved date parsing for crypto transactions from Revolut

@@ -62,10 +62,12 @@ def _action(cls, row: dict) -> Action:

@classmethod
def _datetime(cls, row: dict) -> pendulum.DateTime:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO, the whole _datetime function can look just like this:

parsed = dateutil_parser.parse(row["Date"])
return pendulum.instance(parsed)

with include from dateutil import parser as dateutil_parser at the top of the file

In that way we won't be dependent on any other future date format changes

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I need to give it a try - if it works it would be awesome! 👍

It's really tedious to work out those Revolut date formats...

@MarvinRucinski could you check it out?

Copy link
Copy Markdown
Owner

@pbialon pbialon left a comment

Choose a reason for hiding this comment

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

I did some refactoring since you created the PR (sorry 🙏 ).

If you need help in rebasing it just let me know - I can do it with Claude Code in couple of minutes :)

And thanks for contributing!

Comment on lines +45 to +51
operation_type = cls.OPERATIONS.get(row['Type'])
if operation_type:
return operation_type
for operation_name, operation_type in cls.OPERATIONS.items():
if operation_name in row['Type']:
return operation_type
return None
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'd rather make this whole function a regex matching, instead of that.

'BUY\w+' is OperationType.BUY etc,
'DIVIDEND' is OperationType.DIVIDEND

currency = CurrencyBuilder.build(row['Currency'])
# e.g."-$1,003.01"
amount_row = row['Total Amount']
amount_row = amount_row.strip(str(currency))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I handled it already in #36 :)

@@ -62,10 +62,12 @@ def _action(cls, row: dict) -> Action:

@classmethod
def _datetime(cls, row: dict) -> pendulum.DateTime:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I need to give it a try - if it works it would be awesome! 👍

It's really tedious to work out those Revolut date formats...

@MarvinRucinski could you check it out?

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.

3 participants