Sitelet https://web.archive.org/web/20210724210340/https://github.com/matplotlib/mplfinance/issues/354
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

Change scatter markers edgecolor and/or edgewidth #354

Open
fxhuhn opened this issue Mar 17, 2021 · 11 comments
Open

Change scatter markers edgecolor and/or edgewidth #354

fxhuhn opened this issue Mar 17, 2021 · 11 comments

Comments

@fxhuhn
Copy link

@fxhuhn fxhuhn commented Mar 17, 2021

Hi there,
while playing with the alpha mode (alpha=0.1) I noticed that the marker have a border. Is that a feature, or is there any way to disable it?

image

`

    if df.signal_bull_week.notna().sum() > 0:
        signal_bull_week = mpf.make_addplot( df.signal_bull_week - 1 * offset,
                                    scatter=True,
                                    markersize=40,
                                    marker='^',
                                    alpha=0.1,
                                    color='black')
        add_plots.append(signal_bull_week)

    if df.signal_bear_week.notna().sum() > 0:
        signal_bear_week = mpf.make_addplot(df.signal_bear_week + 1 * offset,
                                     scatter=True,
                                     marker='v',
                                     markersize=40,
                                     alpha=0.1,
                                     color='black')
        add_plots.append(signal_bear_week)

`

@DanielGoldfarb
Copy link
Collaborator

@DanielGoldfarb DanielGoldfarb commented Mar 17, 2021

@fxhuhn
Markus,
Matplotlib "Filled" markers definitely have edges which have both edgecolors and linewidths.
See marker reference for a list of "Unfilled" and "Filled" markers.

The edgecolors and linewidths kwargs to matplotlib.axes.Axes.scatter() are not exposed in Mplfinance. It is an easy change to add these kwargs to mpf.make_addplot() and pass them on to matplotlib. Would love for you to contribute this change to mplfinance. I can guide you through the process if you want.

In the meantime, if you want to make the edges disappear, as a workaround, you can create a custom mpf style that modifies the edgecolor default to none as follows:

s = mpf.make_mpf_style(base_mpf_style='<your chosen style here>',rc={'scatter.edgecolors':None})

All the best. --Daniel

@DanielGoldfarb DanielGoldfarb changed the title Change border color while using alpha mode Change scatter markers edgecolor and/or edgewidth Mar 18, 2021
@fxhuhn
Copy link
Author

@fxhuhn fxhuhn commented Mar 18, 2021 •

Asked a question and got a task. ;-)

If you support me I will be happy to do so. I am a complete beginner in GitHub.

@DanielGoldfarb
Copy link
Collaborator

@DanielGoldfarb DanielGoldfarb commented Mar 18, 2021 •

Markus- Great! The first step is to fork and clone the repository. In case you are not familiar, the basic workflow is this:

  1. fork mplfinance from matplotlib/mplfinance to your account (this makes a copy of mplfinance under https://github.com/fxhuhn)
  2. make a clone of mplfinance from your fork onto your local machine where you will work. (for example, in a terminal session you can run git clone git@github.com:fxhuhn/mplfinance.git; there are other ways too; see documentation or ask).
  3. install the local clone in "editable" mode:
    a. cd into the mplfinance directory that contains setup.py
    b. pip install -e . (notice the dot "." at the end) "editable" means as you make changes to the code they are immediately reflected in the installation of mplfinance, so you don't have to remember to reinstall after each change in order to test.
    c. If you want, you can do this step (installation of local clone) in a python virtual environment. I personally don't bother, and just reinstall the production version (pip install --upgrade mplfinance) when I'm done. But the choice is yours, whatever you are more comfortable with.
  4. when the code is ready, or periodically after one or more commits, you git push the code changes back to your fork.
  5. when all done, you submit a PR (pull request) on GitHub asking the maintainers of mplfinance (that's me) to pull your changes from your fork into the main repository (matplotlib/mplfinance).
  6. If the maintainers request changes, then, for as long as the PR remains open, any additional git push into your fork will automatically feed into the PR, so it is easy to make changes without having to re-submit another PR.

There is documentation on fork and clone here.

Take a look at the code for make_addplot() and the "addplot" section of plot() and let me know if you have any questions where to begin. Both can be found in file src/mplfinance/plotting.py.

Thanks. --Daniel

@fxhuhn
Copy link
Author

@fxhuhn fxhuhn commented Mar 28, 2021

In the meantime, if you want to make the edges disappear, as a workaround, you can create a custom mpf style that modifies the edgecolor default to none as follows:

s = mpf.make_mpf_style(base_mpf_style='<your chosen style here>',rc={'scatter.edgecolors':None})

I tested it with None, 'none' and 'face' but nothing happend.
https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.scatter.html

        s = mpf.make_mpf_style(marketcolors=mc,
                               y_on_right=True,
                               edgecolor='black',rc={'scatter.edgecolors':'face'}
                               )

Do you have any idea what I am doing wrong?

@anushkrishnav
Copy link

@anushkrishnav anushkrishnav commented Apr 20, 2021

Hey I can take and work on the issue if its still open for contributions

@DanielGoldfarb
Copy link
Collaborator

@DanielGoldfarb DanielGoldfarb commented Apr 20, 2021

@anushkrishnav
Anush, As far as I know Markus has not started any work on this. I would suggest using mpf.make_addplot() existing kwarg width for Axes.scatter() kwarg linewidths, and add a new kwarg edgecolors (which in theory can be used for both scatter() and bar()).

@fxhuhn
Markus, sorry for not responding earlier to your question ... Don't know why rc={'scatter.edgecolors':None} did not working for you. It definitely worked fine when I tested it. Pehaps it depends what else is on the plot, possibly working for some styles but not for others?

@fxhuhn
Copy link
Author

@fxhuhn fxhuhn commented May 25, 2021 •

Take a look at the code for make_addplot() and the "addplot" section of plot() and let me know if you have any questions where to begin. Both can be found in file src/mplfinance/plotting.py.

First idea was update mpfstyle:

  • add scatter edgecolors and linewidths to _apply_mpfstyle(style) similar to
    if 'edgecolor' in style and style['edgecolor'] is not None:
        plt.rcParams.update({'axes.edgecolor' : style['edgecolor'] })

Second idea update addplot:

  • add edgecolors and linewidths to _valid_addplot_kwargs()
  • update _addplot_columns

You can see my current work status at https://github.com/fxhuhn/mplfinance

@fxhuhn
Copy link
Author

@fxhuhn fxhuhn commented Jun 13, 2021

@DanielGoldfarb
Copy link
Collaborator

@DanielGoldfarb DanielGoldfarb commented Jun 13, 2021

@fxhuhn
Markus,
One travis issue is that we check and expect every pull request will bump the version number at least one notch.

The other appears to be the final test in https://github.com/matplotlib/mplfinance/blob/master/tests/test_addplot.py#L357

Very busy this week so not sure I will have time to look into why that final test is not matching. Please check and confirm that, other than your change, you have merged in all commits from the current master branch. If that doesn't help, I will look into it when I have time.

(I noticed a week ago, and have been meaning to review, your PR; only, as mentioned, have a lot of stuff on my plate right now so it may be a few days or a week before I can carve out the time).

All the best. An thank you very much for contributing! --Daniel

@fxhuhn
Copy link
Author

@fxhuhn fxhuhn commented Jun 13, 2021 •

@DanielGoldfarb
Danie, my intention was not to stress you.

PR means personal record for me, because it's my first PR and I was not sure if what I did was correct so far.

For me, it seems that I’m on the latest version.
FF7697AB-2631-4FB1-9756-0DE1EBE9F8A1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants