Support for Legends

[ppseverin]

Regarding this question I posted 2 days ago, I started to inspect the code and made some changes. Now I'm able to do this:

mpf.make_addplot(bars[["sma_10","sma_200"]],labels=["sma_10","sma_200"])

Is this useful to the community? What other testing is needed and how do I submit the changes to see if they are approved? Its the first time I change the code of a library I downloaded.

Hope you will find it useful. The results are in the image below.

[DanielGoldfarb]

Hi. Thank you very much for your interest in contributing code to mplfinance.

The standard way to contribute code on GitHub is to use the "fork-clone workflow." I will assume you are somewhat familiar with git itself, but if not, that's fine; just let me know and I will walk you though the steps in more detail.

The fork-clone workflow (click here) is essentially this:

1. You fork the repository (mplfinance). This makes a copy of the repository in your github account
2. You clone your fork of the repository, creating a local copy on your local machine.
3. Make your code changes locally, and test them.
4. git Push your changes back to your fork
5. Submit a pull request to the main repository (https://github.com/matplotlib/mplfinance). This is a request to "pull" the code from your fork, into the main repository.
6. The code gets reviewed and, if acceptable, gets merged from your fork, into the main repository.

Since this is your first time, I suggest the following:

• read the "fork-clone workflow" link posted above.
• before submitting your pull request (but after you have pushed to your fork) contact me and I will review the code in your fork. This can save time if there are any issues with merging the code.

You can contact me either by posting here on this issue, or by emailing me at [email protected]

Please let me know if you have any questions.

All the best. --Daniel

P.S. there is also a little more detail about forking here.

[ppseverin]

Thank you for the datailed steps, Daniel. I will try to follow this instructions and if any question comes up, I'll write you an email. One thing, do I need to run git init before forking or cloning the repository or its not necessary in this case? Sorry the noob question. I have started to use git since like 1-2 days and still learning :)

[DanielGoldfarb]

• git init is not necessary since the repository already exists. (Only use git init when creating a brand new repository).

• fork is a button click on github.com

• git clone <url> is a command run on your local machine to "clone the fork" from github to your local machine.

hth. --Daniel

[ppseverin]

Okay. I think I did it right? Had a little bit of trouble at the beginning, but seems to be okay now. How can I know if I pushed all files right? Will I get any feedback or something?

(push...pull...sometimes a litttle confused with that, but getting used to :) )

Let me know Daniel if its all okay!

[DanielGoldfarb]

You can tell if the push worked by going to your fork and clicking the compare button. Then scroll down and look over the diff and make sure it makes sense in terms of the changes that you expect that you pushed.

I can see your pull request here. It appears you did the push, and PR (pull request) correctly. One thing to be aware of (and this is good) ... the way GitHub PR's work is that as long as the PR is open, any additional pushes (on same branch) to your fork will automatically propagate into the PR. That's good, because if I ask for any changes all you have to do is make those changes and push again ... and those changes will propagate into the existing pull request.

That said, it's late here and I have a very busy day tomorrow, so I may not have a chance to look over the code until Monday. Meanwhile I can tell you this much: The reason the Travis checks are failing on the PR is because one of the checks we put in place checks to make sure that each PR increases the version. You can do this by modifying one line in file _version.py from

version_info = (0, 12, 7, 'alpha', 1)

to

version_info = (0, 12, 7, 'alpha', 2)

If you change the version and git push to your fork again, then hopefully the automated regression tests will all succeed.

[DanielGoldfarb]

Please see code review comments that I have posted on the PR.

[ppseverin]

Hi Daniel,

I just saw the comments you left on github. I did not noticed that the labels needed to be treated as a tuple or list. I agree with you that the labels should be converted to a list if the label is only 1 string. I can add that change and also add the change of using ' ' and not " " (sorry about that).

Regarding the part that you said that I removed something from the code, when submitting the code I changed I noticed a lot of red lines that supposedly I changed. However, I found it strange because I did not delete any line and I guessed that those were deleted lines from before. The process I did was:

1. Fork the repository
2. Clone the repository to my local machine
3. Replaced the cloned files with the files that I had changed, that where in already in my local machine (those files were form the installation files of mplfinance)
4. Commit
5. Push back to my repository
6. Made a pull request

Are these the steps that I should have followed?

How should we proceed?

Best regards, PS

El lun., 19 oct. 2020 a las 14:41, Daniel Goldfarb (< [email protected]>) escribió:

Please see code review comments that I have posted on the PR.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/matplotlib/mplfinance/issues/276#issuecomment-712325715, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOWW5JLDYVZ2BBGNZ3VJGQLSLR26HANCNFSM4SURPZKQ .

[DanielGoldfarb]

Pedro, Thank you for the detailed steps. This is an interesting problem and I learned something new today, which I will share shortly.

The steps are correct except for step 3

3. Replaced the cloned files with the files that I had changed, that where in already in my local machine (those files were form the installation files of mplfinance)

It is a good thing that you were very clear about where the files came from, or I might still be very confused as to how this could have happened. It happens often, in many projects, that the version on pypi.org can differ from the version in the github repository.

This is because one or more Pull Requests may be accepted into the repository before the maintainers decide to release a new version to pypi.org, thus the repository may be any number of commits ahead of the pypi.org installation version.

In the case of mplfinance, presently pypi.org is on version 0.12.7a0, while the repository is at 0.12.7a1 and has the additional enahcnements to the title kwarg. Since you used the files from your installation, those files did not have those changes. But no worries. This is relatively easy to fix. There are two approaches, and you can choose the one you are more comfortable with, but I recommend the second approach because it more fully utilizes the power of git, and it avoids the need to manually re-edit to add your changes again.

1. The first approach is basically to go back in time and re-apply your changes manually instead of copying from your local files. You can go "back in time" by checking out the version of plotting.py from before you applied your changes: git checkout HEAD~1 src/mplfinance/plotting.py. You then manualy edit src/mplfinance/plotting.py and manually apply your changes to that file for the labels enhancement. Then test, commit, and push to your fork, and this will propagate to the pull request.

2. This approach I just learned today after some googling. Essentially two changes were independently made to one base version of plotting.py: (a) your changes for labels, and (b) Teddy Rowan's changes for the title kwarg. Those two independent changes need to be merged together, and the simplest way to do that is to do a "three way merge."
   Effectively it is just a merge of the two enhancements (a) and (b), but in order to correctly merge them, the merge tool needs (as a third file) the "base file" from which each of the two enhancements was derived. You already have your version of plotting.py.
   We can generate the previous version (that you accidentally skipped), and the base version, using the git cat-file command. In order to do this we need to know which commits contain the file version that we want. The previous version (that you skipped) is easy, because we know you made only one commit, so we can use HEAD~1. But to get the base version, we need to know the commit SHA-1. We can get this from git log. But first we need to know how far back that was:
   
   Pypi.org shows that "installed" version of mplfinance was last updated on Aug 9 (see upper right corner of pypi page).
   git log --pretty='%h %cd %an' -10  displays the following:

0d8df09 Sun Oct 18 02:13:06 2020 +0000 Pedro Severin
7b1b722 Mon Sep 14 21:26:38 2020 -0400 Daniel Goldfarb
3af71a8 Sun Aug 16 20:58:45 2020 -0400 Daniel Goldfarb
f5c1338 Sun Aug 16 20:16:45 2020 -0400 TR
1685252 Wed Aug 12 23:02:51 2020 -0400 TR
278e4b4 Sun Aug 9 14:21:48 2020 -0400 Daniel Goldfarb
a1ddfdd Sun Aug 9 14:18:25 2020 -0400 Daniel Goldfarb
f3bbcb7 Fri Aug 7 12:22:51 2020 -0400 Daniel Goldfarb
9b1516f Fri Aug 7 11:52:55 2020 -0400 Daniel Goldfarb
02ba8ac Fri Aug 7 11:52:55 2020 -0400 Daniel Goldfarb

From the above we can see that the version you started with, the "base" version, from Aug 9 is 278e4b4 And the previous version, that you accidentally skipped, that is represented by HEAD~1, is 7b1b722

Now we can generate the skipped version file, and the base version file, and do the three way merge as follows:

git cat-file -p 278e4b4:src/mplfinance/plotting.py > src/mplfinance/plotting.base.py
git cat-file -p 7b1b722:src/mplfinance/plotting.py > src/mplfinance/plotting.skipped.py
git merge-file src/mplfinance/plotting.py src/mplfinance/plotting.base.py src/mplfinance/plotting.skipped.py

src/mplfinance/plotting.py now contains the merged contents.
You can verify this with git status and git diff.

Now commit the merged contents:

git commit -am 're-apply skipped commit for title dict'

Now test. If everything looks good, then git push to your fork and it will propagate into the Pull Request.

I really enjoyed learning this today. I had personally never used git merge-file nor git cat-file so it was fun to learn and try.

I hope that helps. Feel free to do whichever approach with which you are more comfortable. And I very much appreciate your willingness to contribute to mplfinance. I hope you are enjoying and learning from the process.

All the best. --Daniel

[DanielGoldfarb]

By the way, just in case you are not aware: There is a very easy way to install the version of any python package that you are working on. Simply go to the top level source code directory, the one that contains the file setup.py and type the following:

pip install -e .

Note the dot at the end. That is important. This creates an "editable" install (-e) which means that any changes you make to the repository there will be immediately reflected in the installed package. No need to reinstall every time you tweak some code and want to test. I find this very convenient when I am working on a change.

[ppseverin]

Daniel,

Well I see now why the differences. I'm sorry to cause you such trouble, I didn't knew that there could be different versions on github and pypi. Actually, I thought that pypi always had the most recent version of the code. That had some synchronism with github and automatically updates repositories as they were updated in github. I'll pay more attention to that in the future.

It's funny, because I know how to code reinforcement learning methods and some other fancy machine learning stuff (especially for trading), but I don't really know how to use git, which seems to be absolutely basic to many people. I guess that's because I have always worked alone and haven't had the need of using git, until now. So yeah, I enjoy the process of learning new things. Especially if they are useful like git.

Regarding the solution to merge the files, I think that option 1 is probably the fastest, but I would definitely like to try and learn how to do option 2. So, if we are not in a rush I would like to give it a try and learn how to use git properly. If not, then I can just edit manually the code and use another git push.

Let me know what you think.

Best regards, PS

El mar., 20 oct. 2020 a las 17:33, Daniel Goldfarb (< [email protected]>) escribió:

By the way, just in case you are not aware: There is a very easy way to install the version of any python package that you are working on. Simply go to the top level source code directory, the one that contains the file setup.py and type the following:

pip install -e .

Note the dot at the end. That is important. This creates an "editable" install (-e) which means that any changes you make to the repository there will be immediately reflected in the installed package. No need to reinstall every time you tweak some code. I find this very convenient when I am working on a change.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/matplotlib/mplfinance/issues/276#issuecomment-713123170, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOWW5JNKUAS3B2DYT7DXFX3SLXX25ANCNFSM4SURPZKQ .

[DanielGoldfarb]

Pedro,

There is no rush. Take your time. Git, unfortunately, has a steep learning curve. I was using svn and resisted learning git for a long time. When I finally decided to learn git, I learned just a few basic commands and used only those commands for a long time until I felt more comfortable. It is becoming the "defacto" standard for collaboration, and for good reason. It takes a while to learn, but it is very powerful. I find when I'm wondering how to do something with git a little googling usually finds the answer.

Regarding trying new things with git and the repository, as long as you do not push to your fork, if you make a mistake in your local repository you can always delete it (or move it aside) and clone again from your fork. I actually did that several times today (re-cloning your fork) as I was experimenting with differrent approaches to be able to merge the correct versions of the plotting.py file.

All the best. --Daniel

[ppseverin]

Hi Daniel,

I hope that this email finds you well. I did followed the steps of:

git cat-file -p 278e4b4:src/mplfinance/plotting.py > src/mplfinance/ plotting.base.py

git cat-file -p 7b1b722:src/mplfinance/plotting.py > src/mplfinance/plotting.skipped.py

git merge-file src/mplfinance/plotting.py src/mplfinance/plotting.base.py src/mplfinance/plotting.skipped.py

But I noticed that plotting.py now has like ~1800 lines. Maybe it's another noob question of mine, but did I have to previously delete the plotting.py file before merging? Because, if I'm not mistaken, a merge operation should add the differences of plotting.base.py and plotting.skipped.py to plotting.py, right?

And after all this, I should delete plotting.skipped.py and plotting.base.py to only commit plotting.py?

I have a feeling that I did something wrong here :(

Best regards, PS

El mar., 20 oct. 2020 a las 18:14, Daniel Goldfarb (< [email protected]>) escribió:

Pedro,

There is no rush. Take your time. Git, unfortunately, has a steep learning curve. I was using svn and resisted learning git for a long time. When I finally decided to learn git, I learned just a few basic commands and used only those commands for a long time until I felt more comfortable. It is becoming the "defacto" standard for collaboration, and for good reason. It takes a while to learn, but it is very powerful. I find when I'm wondering how to do something with git a little googling usually finds the answer.

Regarding trying new things with git and the repository, as long as you do not push to your fork, if you make a mistake in your local repository you can always delete it (or move it aside) and clone again from your fork. I actually did that several times today (re-cloning your fork) as I was experimenting with differrent approaches to be able to merge the correct versions of the plotting.py file.

All the best. --Daniel

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/matplotlib/mplfinance/issues/276#issuecomment-713143577, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOWW5JOE34LESXN2O2HVBQLSLX4UXANCNFSM4SURPZKQ .

[DanielGoldfarb]

Sounds like it might be OK. You're understanding seems to be correct. I'll take a closer look soon and let you know.

[DanielGoldfarb]

... a merge operation should add the differences of plotting.base.py and plotting.skipped.py to plotting.py, right?

This is correct!

did I have to previously delete the plotting.py file before merging?

Absolutlely not! At the beging of the above operations plotting.py should be the version that is currently in your fork, which is also identical to the version currently in the Pull Request. It contains 963 lines.

And after all this, I should delete plotting.skipped.py and plotting.base.py to only commit plotting.py?

You can delete .skipped and .base if you want to. Certainly they should not be added to the repository. Yes, commit only plotting.py

---

I just reproduced the process as follows:

dino@DINO:~/code/pr277$ git clone [email protected]:ppseverin/mplfinance.git
Cloning into 'mplfinance'...
remote: Enumerating objects: 56, done.
remote: Counting objects: 100% (56/56), done.
remote: Compressing objects: 100% (33/33), done.
remote: Total 2525 (delta 27), reused 35 (delta 19), pack-reused 2469
Receiving objects: 100% (2525/2525), 45.94 MiB | 8.95 MiB/s, done.
Resolving deltas: 100% (1423/1423), done.

dino@DINO:~/code/pr277$ cd mplfinance
/home/dino/code/pr277/mplfinance

dino@DINO:~/code/pr277/mplfinance$ wc -l src/mplfinance/plotting.*
963 src/mplfinance/plotting.py

dino@DINO:~/code/pr277/mplfinance$ git cat-file -p 278e4b4:src/mplfinance/plotting.py > src/mplfinance/plotting.base.py
dino@DINO:~/code/pr277/mplfinance$ git cat-file -p 7b1b722:src/mplfinance/plotting.py > src/mplfinance/plotting.skipped.py
dino@DINO:~/code/pr277/mplfinance$ wc -l src/mplfinance/plotting.*
   953 src/mplfinance/plotting.base.py
   963 src/mplfinance/plotting.py
   965 src/mplfinance/plotting.skipped.py
  2881 total

dino@DINO:~/code/pr277/mplfinance$ git merge-file src/mplfinance/plotting.py src/mplfinance/plotting.base.py src/mplfinance/plotting.skipped.py
dino@DINO:~/code/pr277/mplfinance$ wc -l src/mplfinance/plotting.*
   953 src/mplfinance/plotting.base.py
   975 src/mplfinance/plotting.py
   965 src/mplfinance/plotting.skipped.py
  2893 total

You can see from the above that plotting.py went from 963 lines to 975 lines (by inserting the differences between .base and .skipped, as you mentioned above). The only thing I can think of that may have gone wrong is possibly you had either some other edits to the local src/mplfinance/plotting.py and/or some other commits. Did you try the git log --pretty='%h %cd %an' -10 and did its output look like what I posted above?

At any rate, I reccommend that you go to a clean location (directory) on you system, and start with a fresh clone of your fork (as I did above). It should work then. Let me know. Thanks.

[ppseverin]

Hi Daniel,

I'm sorry to write to you this late. I was quite busy this last two weeks, but now I have some time to finish this git challenge :D.

I was reviewing the code I forked vs the code I merged and the code I finally committed and I noticed that the issue could be this:

• Since I originally committed the files that I installed via pip, the resulting commit was lacking the improvements done by TR.
• Then, I merged both files (cloning the repo in my local PC), but it really got my attention that the resulting script was almost doubling the amount of lines. In fact, the resulting script had twice the number of functions (the ones that I modified plus the original ones).
• Then, I realized that probably the best way to do this is to delete the first fork and perform a new one, delete the cloned repo from my local PC and clone it again, do the changes in the code I am proposing and commit - push them again (and finally do a pull request). Is this the way it should be done?

Off-topic question: I wanted to ask you what kind of relation do you have with trading? Are you an active trader or work for a trading institution? I'm asking because I dream to become a professional algorithmic trader, but sometimes I feel lost with the methods I'm taking. I have spent tons of hours learning to code in python and using neural networks. I actually got some promising results in the market with one of my first reinforcement learning traders, but I feel I'm lacking some knowledge in some points. Any advice?

Best regards, PS

El jue., 22 oct. 2020 a las 23:38, Daniel Goldfarb (< [email protected]>) escribió:

... a merge operation should add the differences of plotting.base.py and plotting.skipped.py to plotting.py, right?

This is correct!

did I have to previously delete the plotting.py file before merging?

Absolutlely not! At the beging of the above operations plotting.py should be the version that is currently in your fork, which is also identical to the version currently in the Pull Request. It contains 963 lines.

And after all this, I should delete plotting.skipped.py and plotting.base.py to only commit plotting.py?

You can delete .skipped and .base if you want to. Certainly they should not be added to the repository. Yes, only commit plotting.py

I just reproduced the process as follows:

dino@DINO:~/code/pr277$ git clone [email protected]:ppseverin/mplfinance.git Cloning into 'mplfinance'... remote: Enumerating objects: 56, done. remote: Counting objects: 100% (56/56), done. remote: Compressing objects: 100% (33/33), done. remote: Total 2525 (delta 27), reused 35 (delta 19), pack-reused 2469 Receiving objects: 100% (2525/2525), 45.94 MiB | 8.95 MiB/s, done. Resolving deltas: 100% (1423/1423), done.

dino@DINO:~/code/pr277$ cd mplfinance /home/dino/code/pr277/mplfinance

dino@DINO:~/code/pr277/mplfinance$ wc -l src/mplfinance/plotting.* 963 src/mplfinance/plotting.py

dino@DINO:~/code/pr277/mplfinance$ git cat-file -p 278e4b4:src/mplfinance/plotting.py > src/mplfinance/plotting.base.py dino@DINO:~/code/pr277/mplfinance$ git cat-file -p 7b1b722:src/mplfinance/plotting.py > src/mplfinance/plotting.skipped.py dino@DINO:~/code/pr277/mplfinance$ wc -l src/mplfinance/plotting.* 953 src/mplfinance/plotting.base.py 963 src/mplfinance/plotting.py 965 src/mplfinance/plotting.skipped.py 2881 total

dino@DINO:~/code/pr277/mplfinance$ git merge-file src/mplfinance/plotting.py src/mplfinance/plotting.base.py src/mplfinance/plotting.skipped.py dino@DINO:~/code/pr277/mplfinance$ wc -l src/mplfinance/plotting.* 953 src/mplfinance/plotting.base.py 975 src/mplfinance/plotting.py 965 src/mplfinance/plotting.skipped.py 2893 total

You can see from the above that plotting.py went from 963 lines to 975 lines (by inserting the differences between .base and .skipped, as you mentioned above). The only thing I can think of that may have gone wrong is possibly you had either some other edits to the local src/mplfinance/plotting.py and/or so other commits. Did you try the git log --pretty='%h %cd %an' -10 and did its output look like what I posted above?

At any rate, I reccommend that you go to a clean location (directory) on you system, and start with a fresh clone of your fork (as I did above). It should work then. Let me know. Thanks.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/matplotlib/mplfinance/issues/276#issuecomment-714870827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOWW5JIYEUHKSJXNDZLEOMDSMDUAVANCNFSM4SURPZKQ .

[DanielGoldfarb]

Then, I realized that probably the best way to do this is to delete the first fork and perform a new one, delete the cloned repo from my local PC and clone it again, do the changes in the code I am proposing and commit - push them again (and finally do a pull request). Is this the way it should be done?

Pedro- It is fine to do what you have outlined above. Sometimes when repo issues get complicated it is easier to just "start from scratch." I've do that from time to time. This is what I meant above when I suggested one approach would be to ...

re-apply your changes manually.

So, yes, go ahead and do that. It should work. I'm not sure what GitHub will do about the existing PR. You can always click "Close pull request" at the bottom of the PR; (maybe even close it before deleting and recreating your fork).

Regarding trading and career stories, I will send you an email and we can start an email thread specifically on that topic.

No worries about being busy. Busy is good. Busy is a blessing. All the best. --Daniel

[ppseverin]

Daniel,

The changes are ready. This time I'll wait for you before doing the pull request.

Best regards, PS

El mié., 4 nov. 2020 a las 20:39, Daniel Goldfarb ([email protected]) escribió:

Then, I realized that probably the best way to do this is to delete the first fork and perform a new one, delete the cloned repo from my local PC and clone it again, do the changes in the code I am proposing and commit - push them again (and finally do a pull request). Is this the way it should be done?

Pedro- It is fine to do what you have outlined above. Sometimes when repo issues get complicated it is easier to just "start from scratch." I've do that from time to time. This is what I meant above when I suggested one approach would be to ...

re-apply your changes manually.

So, yes, go ahead and do that. It should work. I'm not sure what GitHub will do about the existing PR. You can always click "Close pull request" at the bottom of the PR; (maybe even close it before deleting and recreating your fork).

Regarding trading and career stories, I will send you an email and we can start an email thread specifically on that topic.

No worries about being busy. Busy is good. Busy is a blessing. All the best. --Daniel

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/matplotlib/mplfinance/issues/276#issuecomment-722033035, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOWW5JOMCXT4P6PDG6FB77LSOHQ2BANCNFSM4SURPZKQ .

[DanielGoldfarb]

OK. Thanks. I'll take a look tomorrow.

[DanielGoldfarb]

My apologies for not updating this issue sooner. I did some testing of the code, and experimented with a lot of various use cases. It turns out this is not a simple as we thought. While it is relatively easy to get legens/legend-labels working well for some use-cases, it turnsout that (as implemented) they would not work in many other use cases. This is because legends are dependent on having access to each matplotlib Artist object that you want to label. Unfortunately, the way mplfinance is currently structured, some of the Artist objects are readily available while other are not.

This is not to say that we should not do this. On the contrary, based on my experiements, I have some ideas on how to revamp the code to make legends easier, but the revamping may take some time. That is why I decided to first focus on some easier enhancements. (You might think at first that legends would be easy, but alas.) I expect a few more weeks of working on other enhancement requests, and then I plan to come back to this.

[Rickaym]

Any updates on this? Thanks.

[hubbs5]

Any updates on this? Thanks.

See this example. I didn't find anything in the official docs, but this comment helped immensely!