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

Handle initial OBV value & Handle 0's #347

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kwuasimoto
Copy link

@Kwuasimoto Kwuasimoto commented Sep 13, 2024

While writing my own ta library, I noticed a discrepancy between my OBV values and ta's.

I did some digging and noticed 2 easy to fix issues. I've ran the unit tests and everything still works 👍

Issues:

  1. For the initial volume value, we apply 0 because there is no history to confirm whether the movement was up or down.
  2. When the current days close is equal to the previous, we apply 0 because there was no up or down movement.

Fixes:

  1. Initialize an empty zeros array with the same shape as the volume with np.zeros_like()
  2. Use masks to insert positive and negative volume values into the zeros array.

Reasoning:

  1. We can see in the screenshot below, that the initial day has no obv value. This is effectively 0
    Screenshot 2024-09-13 011141
  2. In the Investopedia formula, we can see if close = close_prev then 0, this is not handled properly in the current implementation.
    Screenshot 2024-09-13 003324

Tests:
Screenshot 2024-09-13 011442

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.

1 participant