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

added vlines feature to plotting.py #570

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

deepchandra02
Copy link

Hello, this is an enhancement request based off of the request: #513

Please review the code and let us know if it is acceptable for merge! This is for our course project, I hope this helps :)

Copy link
Collaborator

@DanielGoldfarb DanielGoldfarb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. You need to remove the build and dist directories from the repository. (build and dist directories should never be committed to a repository).
  2. test.py
    a. needs a better name
    b. there should not be two copies of the same file in two different locations.
    c. if test.py is intended to be part of the regular regression tests, then it should be modeled after, and follow a similar pattern as, the other existing test files.
  3. Both pr569_testing.ipynb and testing_issue551.ipynb should be moved into the following directory: examples/scratch_pad/issues/. Also, once you move them there, you may have to modify the input data paths, so that those notebooks still run in this new location.

Please make the above mentioned changes in order to continue with this Pull Request.
Thank you.

@@ -492,6 +497,7 @@ def plot( data, **kwargs ):
else:
panels = _build_panels(fig, config)
axA1 = panels.at[config['main_panel'],'axes'][0]
warnings.warn(f"this type is {type(axA1)}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this line of code??

@@ -407,7 +411,8 @@ def plot( data, **kwargs ):
moving averages, renko, etc.
Also provide ability to plot trading signals, and/or addtional user-defined data.
"""

import sys
sys.stdout = sys.__stdout__
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these two lines of code??

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

Successfully merging this pull request may close these issues.

4 participants