MadeInPierre / finalynx

A minimalistic companion (CLI & web) to organize your investment portfolio, simulate its future, and reach your life goals.
https://finalynx.readthedocs.io
GNU General Public License v3.0
68 stars 13 forks source link

Values are wrong for non-EUR holdings #54

Closed Varal7 closed 1 year ago

Varal7 commented 1 year ago

Problem: The value returned by the Finary API in the field "current_value" corresponds to the currency under ["fiat"]["symbol"] in the same object.

If a user has non-EUR lines, the values displayed will be wrong.

Proposed solution:

Use display_current_value instead of current_value.

One option is to query the user's preferences at /users/me, i.e. ff.get_user_me, under 'ui_configuration.display_currency'.

For example:

python -m finary_api me | jq '.result["ui_configuration"]["display_currency"]'

{
  "id": 1,
  "name": "Euro",
  "code": "EUR",
  "symbol": "€",
  "correlation_id": "6246d83d2a89de8c1452c622",
  "logo_url": "https://cdn.finary.com/000terminal/currencies/logos/Euro EUR.png"
}

Then, instead of using current_value, use display_current_value.

For example, line 222 of fetch_finary.py becomes

        _process("Checkings", ff.get_checking_accounts, lambda e: (e["name"], e["id"], e["fiats"][0]["display_current_value"]))

And _render_currency uses the currency symbol from the user preferences.

Alternatives:

Keep track of the tuple (amount, current) for each line and apply a conversion rate.

MadeInPierre commented 1 year ago

Hi! Thanks for your detailed issue, I have prepared for adding multiple currencies later but have not implemented anything yet as my portfolio only includes euros for now.

I'd imagine a solution in three steps:

My philosophy is to keep Finalynx as independent as possible from Finary's API due to its current non-official state (who knows, maybe they'll shutdown the API for external users one day). Hence, I would prefer the user to set the currency manually for each Line in their configuration file. Optionally, we could improve Finary's fetching process to auto-set as many fields as possible, but this is not implemented yet.

While it could also be useful to apply the conversion rate ourselves, I don't think it's worth the time for now.

What do you think of this implementation?

MadeInPierre commented 1 year ago

If a Folder contains children with multiple currencies, what should the behavior be?

MadeInPierre commented 1 year ago

I added basic support for multiple currencies (described in #55 ), let me know if you have improvement ideas!

Varal7 commented 1 year ago

I see you have changed current_value to display_current_value. With that version of the code, I get a consistent display when configuring my Finary account to use euros. This is the step in the right direction because previously, I could not get any consistent readings.

However, if I change the display currency to something else than euros, it will update the display_current_value but not the currency symbol. You can reproduce by going in settings, "My account", "Currency" and choose something that is not euros.

This is the code I'm currently using to display my tree:

from finalynx import Portfolio, Assistant
portfolio = Portfolio() 
Assistant(portfolio).run()

I am not sure what you did to handle the currency symbol, but it's not working for me. I might take a look this weekend.

MadeInPierre commented 1 year ago

Hi @Varal7,

My philosophy is to keep Finalynx as independent as possible from Finary's API due to its current non-official state [...]. Hence, I would prefer to let the user set the currency manually for each Line in their configuration file.

I added a very basic support which lets you manually specify the currency symbol for each line. Have you tried setting the currency with:

from finalynx import Portfolio, Assistant
portfolio = Portfolio(currency="$")
Assistant(portfolio).run()

I have just tried setting my account to USD and the amounts are indeed different, you just need to set the correct currency symbol for each line (or at the portfolio level to set all lines at once). Please see #55's description, let me know if you have any questions or improvement ideas!

Optionally, we could improve Finary's fetching process to auto-set as many fields as possible, but this is not implemented yet.

If you would prefer to have an automatic setup by using the data available in Finary's response, I'd be happy to accept a PR!

Varal7 commented 1 year ago

Thank you for your response. Yes, that makes sense, I understand the logic of that code better now. I've tried setting the currency for the whole portfolio with "$", and it correctly calls self.set_children_attribs(asset_class, perf, currency) (line 75 of folder.py)

But when it comes back up in get_currency, it's all euros.

Will debug and report here.

Varal7 commented 1 year ago

Ok, I think it's because add_child acts after the __init__ is over, so the new children are not affected by the set_children_attribs call.

MadeInPierre commented 1 year ago

Oh right! My apologies, I didn't think about Lines created from Finary's fetching process when they are not declared in the main portfolio. These lines are called "orphans" and are added just as a reminder so you don't forget to include them in your Portfolio structure (fetch_finary.py line 303):

# Attach it to root if the line is absent (unless ignored)
if not self.ignore_orphans:
    node_child.add("[yellow]WARNING: This line did not match with any envelope, attaching to root")
    self.portfolio.add_child(Line(key, amount=amount))

Once you set your own structure inside the Portfolio("My Portfolio", currency="$", children=[...]) instance, you shouldn't get this issue anymore. I'll think about a fix and apply it soon.

Varal7 commented 1 year ago

Yes, I am guilty of not having created a proper configuration. Now that this #56 fixes my currency issue, I can fully adopt finalynx and spend the time creating a proper structure.

MadeInPierre commented 1 year ago

Perfect! Thanks for the fix and for your interest in this fun project of mine! I'd be happy to discuss about future features, feel free to open issues and PRs :smile:

If there are people willing to use this project, I might spend some time writing better user guides to explain all the little features.