Feature/mstarpy

[Mael-J]

Description

• Summary of the change / bug fix.
• Link # issue, if applicable.
• Screenshot of the feature or the bug before/after fix, if applicable.
• Relevant motivation and context.
• List any dependencies that are required for this change.

How has this been tested?

• Please describe the tests that you ran to verify your changes.
• Provide instructions so we can reproduce.
• Please also list any relevant details for your test configuration.

• Make sure affected commands still run in terminal
• Ensure the SDK still works
• Check any related reports

Checklist:

• Update our Hugo documentation following these guidelines.
• Update our tests following these guidelines.
• Make sure you are following our CONTRIBUTING guidelines.
• If a feature was added make sure to add it to the corresponding scripts file.

Others

• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.

Fix #3975

[hjoaquim]

@Mael-J this was great work, thanks for caring enough to the point of almost rebuilding the whole functionality for the funds menu!

The menu is now usable after some months.
There are a couple of improvements that can be done, but those are out of the scope of this PR - I'll be adding it on the issues tab, so we can work on it later on.

I don't have any big comments to do, as the code was clean and understandable.
I've done some minor changes that I want to know if you agree with. If yes, for me, it's approved.

Once again, great work 🚀

[jmaslek]

I have a small comment:

The functions

│     alswe              display fund allocation (sector, country, holdings)             [Avanza]                                                                                   │
│     infoswe            get fund information       

Should only show when country selected is sweden

[jmaslek]

Im also curious about this. If I look at vfiax, which is trading at around $380, this says 35,000. Is there a factor of 100 off?

[hjoaquim]

changed the swe functions here: 31292d7 to address @jmaslek good point
also, changed a bit of the behavior, so it's now getting the data using the ISIN instead of the name, which works perfectly fine with the new load data source

[jmaslek]

changed the swe functions here: 31292d7 to address @jmaslek good point also, changed a bit of the behavior, so it's now getting the data using the ISIN instead of the name, which works perfectly fine with the new load data source

So now this doesnt work

                                                                                                                                                                          │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── OpenBB Terminal v2.3.1 (https://openbb.co) ─╯
2023 Feb 02, 12:19 (🦋) /funds/ $ load vfiax

0 funds found whith the term vfiax
Error: 0 funds found with the term vfiax

No funds were loaded.

[hjoaquim]

2023 Feb 02, 18:06 (🦋) /funds/ $ country united_states

United_States selected.

2023 Feb 02, 18:06 (🦋) /funds/ $ load vfiax

The fund Vanguard 500 Index Fund Admiral Shares - 52.8.VFIAX (FOUSA00L8W) was successfully loaded.

2023 Feb 02, 18:06 (🦋) /funds/ $

Did you select the country @jmaslek ?
Looks like it's not a mandatory arg for the load but if it's empty the results are not as good / inexistant.
Maybe @Mael-J can add some input on this? Can be as simple as force to always select country before using load - if that makes sense.

[jmaslek]

2023 Feb 02, 18:06 (🦋) /funds/ $ country united_states

United_States selected.

2023 Feb 02, 18:06 (🦋) /funds/ $ load vfiax

The fund Vanguard 500 Index Fund Admiral Shares - 52.8.VFIAX (FOUSA00L8W) was successfully loaded.

2023 Feb 02, 18:06 (🦋) /funds/ $

Did you select the country @jmaslek ? Looks like it's not a mandatory arg for the load but if it's empty the results are not as good / inexistant. Maybe @Mael-J can add some input on this? Can be as simple as force to always select country before using load - if that makes sense.

yup. totally forgot the country. We can either make that required ('no country selected please do so'), or default to united_states and add 'no funds found for {country}' on the load if it fails

[Mael-J]

@jmaslek historical data come from the API performance (https://www.morningstar.com/funds/xnas/vfiax/performance). It is the total performance of the funds.

I created a version 0.0.4 of mstarpy with a method nav which get the actual nav of the funds (https://www.morningstar.com/funds/xnas/vfiax/chart)

I can change the plot and display the actual nav. What do you think ?

[Mael-J]

@jmaslek @hjoaquim I don't know why for some funds the country is mandatory. Selecting a country before load method is good idea.

[jmaslek]

@jmaslek historical data come from the API performance (https://www.morningstar.com/funds/xnas/vfiax/performance). It is the total performance of the funds.

I created a version 0.0.4 of mstarpy with a method nav which get the actual nav of the funds (https://www.morningstar.com/funds/xnas/vfiax/chart)

I can change the plot and display the actual nav. What do you think ?

I think the NAV makes more sense. (As someone invested in vfiax, I wish it were that high!)

[hjoaquim]

@luqmanbello should we worry with these actions?: license/snyk and security/snyk
When I try to see details, I get: "Unable to display this organization"

[jmaslek]

@luqmanbello should we worry with these actions?: license/snyk and security/snyk

When I try to see details, I get: "Unable to display this organization"

You can ignore them for now.