Akshat111111 patch 2 #374
Akshat111111 patch 2 #374
Conversation
Calculating Pivot points directly without using rolling and series objects for faster computation.
Update pivots_points.py
Removing Hardcoding values and repeated indexing , and implementing Numpy functions
| for i in range(1, levels + 1): | ||
| data[f"r{i}"] = 2 * data["pivot"] - dataframe['low'] | ||
| data[f"s{i}"] = 2 * data["pivot"] - dataframe['high'] |
There was a problem hiding this comment.
I'm not sure this is producing the same results, actually (i'm pretty sure it will not).
We used to look at a rolling price (for all 3 necessary units) - which seems to have been removed completely - which will automaticall also completely change results.
I'm not per se against it - but some explanation about your thoughts (why do it this way instead of the other) will for sure be necessary to accept this.
There was a problem hiding this comment.
But I have implemented using Numpy function,So it should work
There was a problem hiding this comment.
err ... no ?
what i mean is - the prior calculation of high and low was using a rolling mean - in pandas / numpy terms something around high = dataframe['high'].rolling(timeperiod).mean()
not using qtpylib here is certainly preferred - and as said above, i'm also not per se against not using rolling averages (and instead use the price directly) - but i'd like to understand the reason for that change (if it was intentional ...).
| max2_idx = np.where(x == max2)[0][0] | ||
| min2_idx = np.where(x == min2)[0][0] | ||
|
|
||
| maxslope = (max1 - max2) / (max1_idx - max2_idx) | ||
| minslope = (min1 - min2) / (min1_idx - min2_idx) | ||
| a_max = max1 - (maxslope * max1_idx) | ||
| a_min = min1 - (minslope * min1_idx) | ||
| b_max = max1 + (maxslope * (len(x) - max1_idx)) |
There was a problem hiding this comment.
I don't see why we would remove the comments (not just on the highlighted lines).
that seems to make following (and understanding) this code quite more complicated (it's not immediately clear what a_max is supposed to contain - for example), especially for someone not 100% familiar with how this code is supposed to work.
Please revert / keep the comments.
There was a problem hiding this comment.
I will readd the comments in order to make it beginner friendly
| x = dataframe[field] | ||
| import numpy as np | ||
| import pandas as pd | ||
| import matplotlib.pyplot as plt |
There was a problem hiding this comment.
Technical doesn't depend on matplotlib - and we don't intend to.
The way it was previously made this optional - where the code would fail if matplotlib isn't installed.
Now it's failing all the time unless we depend on matplotlib.
Please keep the matplotlib import where it was.
There was a problem hiding this comment.
Sure, I will make the desired changes and then redirect to you.
|
Going to close this one - as it appears abandoned - and in reality, is changing way too much at once (often in a odd way) to safely accept without potentially changing results drastically. |
Removing Hardcoding values and repeated indexing, Implementing Numpy fxn