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

Export Account Transactions has wrong tax #141

Open
ralfescher opened this issue Nov 10, 2024 · 11 comments
Open

Export Account Transactions has wrong tax #141

ralfescher opened this issue Nov 10, 2024 · 11 comments
Labels
bug Something isn't working

Comments

@ralfescher
Copy link

ralfescher commented Nov 10, 2024

Hi,

I am a beginner in Python programming and managed to get this (great) program running under a virtual Python 3.9 on my new Mac mini.

While exporting the transactions dl_docs, I noticed that the taxes were incorrectly displayed.

Output: 2024-11-01;Interest;187.16;Zinsen;;;;-6,704
The real value is: 187,16€ and 67,04€ tax (instead of 6704€).

Additionally, I have the problem of importing .csv files into Excel on my Mac (German locale)
(comma/period and UTF-8 -> Mac-Roman).
Would it make sense to offer a specific option for this?

Additional question: Does it make sense to get this app running under Python 3.12 as well? (As mentioned, I’m new to Python).

Thanks
Ralf

@ralfescher ralfescher added the bug Something isn't working label Nov 10, 2024
@ralfescher ralfescher changed the title [BUG] Export Account Transactions has wrong tax Export Account Transactions has wrong tax Nov 10, 2024
@pinzutu
Copy link
Contributor

pinzutu commented Nov 10, 2024

Hi, you can change pytr's csv locale using the export_transactions --lang argument. Excel should also be able to parse semicolumn separated csvs.

Furthermore, could you share the (redacted) json entry of the event with the erroneous tax (all_events.json)?

@msdn65
Copy link

msdn65 commented Nov 11, 2024

In account_transactions.csv I also get wrong or malformatted tax:
2024-11-01;INTEREST;146.51;Zinsen;;;;-3,137

snippet from all_events.json:
{
"title": "Transaktion",
"data": [
{
"title": "Angesammelt",
"style": "plain",
"detail": {
"text": "€177.88",
"type": "text"
}
},
{
"title": "Steuern",
"style": "plain",
"detail": {
"text": "€31.37",
"type": "text"
}
},
{
"title": "Gesamt",
"style": "plain",
"detail": {
"text": "€146.51",
"type": "text"
}
}
],
"type": "table"
},

@ralfescher
Copy link
Author

ralfescher commented Nov 12, 2024

I have the same situation: the values in the JSON look correct (as with @msdn65)
Btw. the command pytr export_transactions does not export the interests to the csv file.

@pinzutu
Copy link
Contributor

pinzutu commented Nov 14, 2024

I see and yes, this event is not yet supported. To add support and for validating potential changes, please share the full json event entry with unaltered keys, nesting, and value formatting.

@SegFaulty
Copy link

SegFaulty commented Nov 17, 2024

Hi, you can change pytr's csv locale using the export_transactions --lang argument.

chnaging the language does not change the outcome, only the format ... tax is wrong

transaction-de.csv:2024-11-01;INTEREST;224,34;Zinsen;;;;-8.035
transaction-en.csv:2024-11-01;INTEREST;224.34;Zinsen;;;;-8,035

80.35 would by correct

BUT I digged deeper:

all_events.json: in locale "en"

             {
              "title": "Steuern",
              "style": "plain",
              "detail": {
                "text": "€80.35",
                "type": "text"
              }
            },

is parsed here (I think) in /pytr/event.py in locale "de":

# Iterate over dicts containing tax information and parse each one
            for taxes_dict in taxes_dicts:
                parsed_taxes_val = cls._parse_float_from_detail(taxes_dict, "de")
                if parsed_taxes_val is not None:
                    return parsed_taxes_val

BUT "text": "€80.35", is not in locale "de" its 80.35 so cls._parse_float_from_detail(taxes_dict, "en") will fix it?

derterz pushed a commit to derterz/pytr that referenced this issue Dec 28, 2024
…g#141)

It seems the "Steuer" value can either be represented as "€11.14" or as
"11,14 €" string.

The "€11.14" representation was misinterpreted as 1114.
@derterz
Copy link
Contributor

derterz commented Dec 28, 2024

I ran into exactly the same problem. After looking into #125 and #126 I wrote a small test for the known string formats. The pull request contains a simple format detection heuristic that works for nicely for the known examples.

rtfpessoa pushed a commit to rtfpessoa/pytr that referenced this issue Jan 3, 2025
…g#141)

It seems the "Steuer" value can either be represented as "€11.14" or as
"11,14 €" string.

The "€11.14" representation was misinterpreted as 1114.

Signed-off-by: Rodrigo Fernandes <[email protected]>
NiklasRosenstein added a commit that referenced this issue Feb 11, 2025
…153)

* Fix: Sometimes Value of tax is too large by a factor of 1000 (#141)

It seems the "Steuer" value can either be represented as "€11.14" or as
"11,14 €" string.

The "€11.14" representation was misinterpreted as 1114.

* Convert parsing test into a plain function instead of a subclass

Using pytest to run the tests does not require the tests to be a subclass
of unittest.TestCase.

* ruff check --fix

---------

Co-authored-by: Ingo Massmann <[email protected]>
Co-authored-by: Niklas Rosenstein <[email protected]>
@MedAzizKhayati
Copy link

MedAzizKhayati commented Feb 12, 2025

After the fixes, the following bug started to occur:

{
          "title": "Transaktion",
          "data": [
            {
              "title": "Anteile",
              "detail": {
                "text": "10,002",
                "trend": null,
                "action": null,
                "displayValue": null,
                "type": "text"
              },
              "style": "plain"
        ...
}

The value here is clearly German, however it's being parsed to 10002 instead of 10.002.

-------- SUGGESTED FIX ---------

    @staticmethod
    def _parse_float_from_detail(elem_dict: Dict[str, Any], preferred: Optional[str] = None) -> Optional[float]:
        unparsed_val = elem_dict.get("detail", {}).get("text", "")
        parsed_val = re.sub(r"[^\,\.\d-]", "", unparsed_val)

        # Try the locale that will fail more likely first.
        if "." not in parsed_val:
            locales = ("en", "de")
        else:
            locales = ("de", "en")

        if preferred and unparsed_val == parsed_val and preferred in locales:
            locales = (preferred, locales[0] if preferred == locales[1] else locales[1])

        try:
            parsed_val = float(parse_decimal(parsed_val, locales[0], strict=True))
        except NumberFormatError:
            try:
                parsed_val = float(parse_decimal(parsed_val, locales[1], strict=True))
            except NumberFormatError:
                return None
        return None if parsed_val == 0.0 else parsed_val

This way, the function user can prefer a locale if there is change where both locale can work. In my example where "10,002" can both be considered as German or American format, the user can prefer "de" and "de" will be used instead. The preferred language is only taken into consideration if after the regex substitution the parsed value and unparsed one stays the same.

@derterz
Copy link
Contributor

derterz commented Feb 12, 2025

It seems this is really a mess, we have 10,002 which is should result in 10.002 and from #125 we have 9.400 which should result in 9400. To my understanding English and German values can be wildly mixed inside the responses. How is the user supposed to set the proper preferred value?
Maybe we can use some context information? The total value of a transaction is should typically be between 1€ and 100.000€.

@MedAzizKhayati
Copy link

MedAzizKhayati commented Feb 13, 2025

We can do that through context as you mentioned or so this way pytr/event.py#L240:

 locales = ["en" if e["title"] == "Aktien" else "de" for e in shares_dicts + fees_dicts]
for key, elem_dict, locale in zip(titles, shares_dicts + fees_dicts, locales):
    return_vals[key] = cls._parse_float_from_detail(elem_dict, locale)

@RealCLanger
Copy link
Collaborator

Please check whether my change in #193 could help here.

@NiklasRosenstein
Copy link
Collaborator

FYI #153 applied @MedAzizKhayati's suggestion, but the link to this issue was never established.

CoLdIcE42 pushed a commit to CoLdIcE42/pytr that referenced this issue Feb 23, 2025
…g#141) (pytr-org#153)

* Fix: Sometimes Value of tax is too large by a factor of 1000 (pytr-org#141)

It seems the "Steuer" value can either be represented as "€11.14" or as
"11,14 €" string.

The "€11.14" representation was misinterpreted as 1114.

* Convert parsing test into a plain function instead of a subclass

Using pytest to run the tests does not require the tests to be a subclass
of unittest.TestCase.

* ruff check --fix

---------

Co-authored-by: Ingo Massmann <[email protected]>
Co-authored-by: Niklas Rosenstein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants