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
65 stars 13 forks source link

Crypto Missing #42

Closed yovanoc closed 1 year ago

yovanoc commented 1 year ago

Il n'y a pas du tout la partie crypto dans les keys proposées, est-ce normal ?

MadeInPierre commented 1 year ago

Hello ! Je n'ai pas encore de cryptos donc aucun moyen de voir ce que retourne l'API pour l'ajouter :shrug: De manière générale, ma méthode est d'avancer sur les features en fonction de mon portefeuille (qui aura bien des cryptos un jour ou l'autre) en essayant de garder une approche extensible. J'accepte avec plaisir toutes contributions pour étendre ce projet pour le faire marcher avec vos situations !

N'hésite pas si tu aimerais que je t'explique comment ajouter les cryptos, l'inspiration peut démarrer ici :slightly_smiling_face:

yovanoc commented 1 year ago

Ok, I never done python coding but can try. How can we launch the examples/demo script to use the current state of the code? when I try to launch it that way it gives me this:

image

and that's what I tried

image
MadeInPierre commented 1 year ago

Weird, the project should be stable. The latest version uses global imports (without dots, meaning finalynx must be installed globally in order to work properly):

image

Some ideas:

You should be able to simply run the demo without any code change:

python examples/demo.py

Let me know if that helps! Maybe the initial setup tutorial could be useful too. Let's try to setup your local project first, and then I'll try to give you my best advice on where to start to add cryptos. If the setup tutorial isn't clear enough, I'd be happy to explain it further (Python is awesome but it can be confusing sometimes ^^)

yovanoc commented 1 year ago

Ok mb I didn't know the pip install -e . to install it from the project and allow me to modify it and test.

I've started the crypto part and I think I will open a PR today. thanks

Just, I really need to do a pr to the finary_api repo to have this or I can access it other way?

image

because when I do this in this repo:

image

it is not available but doing this thing make it work

MadeInPierre commented 1 year ago

Great!

I'll take a look in a moment, ideally you shouldn't need to modify finary_api but I've never tried using cryptos. Be right back!

EDIT: Also, I heavily modified finary_fetch.py yesterday to fix #41, please make sure you are up to date to avoid conflicts

yovanoc commented 1 year ago

Yeah already updated it. Don't worry for git things I'm a pro' developer, just never coded in Python 😂

MadeInPierre commented 1 year ago

Got it sorry x) You'll teach me one day then :stuck_out_tongue:

Several options:

  1. Make a PR in finary_api with what you suggested, looks like the author forgot it or isn't using it in command-line (__main__.py is called when you do python -m mypackage).
  2. Specify the location of the method by using:
    import finary_api.user_portfolio
    ...
    finary_api.user_portfolio.get_portfolio_cryptos(session)
  3. Looks like this method only has one line, simply use the shortcut directly (get_portfolio is available in __main__):
    ff.get_portfolio(session, "cryptos")

EDIT: Forget about the 3rd option, looks like there's another method with the same name that does something different, so option 2 is fine.

yovanoc commented 1 year ago

Got it sorry x) You'll teach me one day then 😛

What do you mean? I didn't want to be rude saying that.

Otherwise, thank you option 2 is good !

MadeInPierre commented 1 year ago

Sure no worries! I simply meant I'd be happy to learn from you since this is literally my first public repo, there's surely plenty of stuff to improve here and I'd love to know about it ^^