The failures are all PEP8, mostly whitespace, plus some overly long lines.
It would be a good idea to configure your editor to strip all trailing whitespace when saving files.

attn: @jdgd1

Sorry, fixed the typos and whitespaces(hopefully).

shouldn't we be calling this "edgecolor" in order to follow existing mpl nomenclature?

👍 To matching the naming convention.

I suggest a small change to the zorder strategy here. The basic point is that the patch should go above the line instead of below. Since the two go together, however, they should have similar zorders so that the user can easily plot a line (default zorder 2) that lands on top of both or below both. Furthermore, one wants both candlestick zorders to be above the default patch zorder (1), so that the candlesticks will appear on top of bar plots and the like. Two options for the zorders of the candlestick components:

1. line 1.9, patch 1.91 so that a default independent line will land on top of the candlesticks
2. line 2.1, patch 2.11 so that a default independent line will land below the candlesticks

I recommend option 2 above. I'm guessing that the use case would be to plot something like a scaled stock index as a line or symbols together with the candlestick plot, and that in such a case the index line is a background, and will look best behind the candlesticks. @lifetime42, do you agree, and if so are you available to make that change? It would be nice to get this resolved and merged.

I think having the average in front or in the back depends on wether the
indicators are "supporting" the chart picture or whether the moving
averages (or any other) are to be looked at directly, eg. at crossing
points. could make an argument for both.ill have a look into it

2016-05-22 1:36 GMT+02:00 Eric Firing notifications@github.com:

I recommend option 2 above. I'm guessing that the use case would be to
plot something like a scaled stock index as a line or symbols together with
the candlestick plot, and that in such a case the index line is a
background, and will look best behind the candlesticks. @lifetime42
https://github.com/lifetime42, do you agree, and if so are you
available to make that change? It would be nice to get this resolved and
merged.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5160 (comment)

On 2016/05/23 11:36 AM, lifetime42 wrote:

I think having the average in front or in the back depends on wether the
indicators are "supporting" the chart picture or whether the moving
averages (or any other) are to be looked at directly, eg. at crossing
points. could make an argument for both.ill have a look into it

Thanks. It's just a question of which is likely to be most common,
particularly for less-experienced users, and therefore should be the
default. It will always be possible to chose the alternative via an
explicit zorder kwarg to the subsequent plot commands.

The default should be to have the indicators in the front.
Either they are comapred to each other or to the bars for interpretation.

2016-05-23 23:58 GMT+02:00 Eric Firing notifications@github.com:

On 2016/05/23 11:36 AM, lifetime42 wrote:

I think having the average in front or in the back depends on wether the
indicators are "supporting" the chart picture or whether the moving
averages (or any other) are to be looked at directly, eg. at crossing
points. could make an argument for both.ill have a look into it

Thanks. It's just a question of which is likely to be most common,
particularly for less-experienced users, and therefore should be the
default. It will always be possible to chose the alternative via an
explicit zorder kwarg to the subsequent plot commands.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5160 (comment)

matplotlib.finance is no more (it has been moved to an external library, mpl_finance: https://github.com/matplotlib/mpl_finance).

Feel free to reopen the PR in that repo.