Unit test calculate_vwap()

[DanilYachmenev]

There is a calculate_vwap() in im_v2/common/data/transform/transform_utils.py (link) that is being used in amp/research_amp/cc/notebooks/master_tradability_analysis.py and amp/research_amp/cc/notebooks/Master_Analysis_CrossSectionalLearning.py

We need to add unit tests using synthetic test data. Create a simple short dataframe with 2 columns: 1 for price and 1 for volume, use a pd.Timestamp index.
Run the function on it and compare the outcome with string signature (for ref see tests in amp/helpers/test/test_hpandas.py e.g. Test_merge_dfs1)

Add 2 unit tests:

• No resample_kwargs
• With resample_kwargs

See unit test doc to follow the code style

Assigning @samarth9008 to re-distribute

FYI @gpsaggese

[samarth9008]

@DanilYachmenev Thanks for the issue

[aish-nidhi]

Thanks @samarth9008 for assigning the issue.
I will be working on the synthetic data set for the test today and will start with the no resample_kwargs test.

[gpsaggese]

@aish-nidhi feel free to post a plan or do a draft PR (e.g., only a sketch/skeleton, we won't expect any convention to be followed, you just have to mark it as draft and ask for a architectural review).
The worst case scenario is misinterpreting the specs and come back with something correct but different than was it requested.

[aish-nidhi]

Sure. Will be doing the draft PR based on my understanding of the issue.Thanks & Regards,Aishwarya NidhiOn 15 Apr 2024, at 4:21 PM, GP Saggese ***@***.***> wrote: @aish-nidhi feel free to post a plan or do a draft PR (e.g., only a sketch/skeleton, we won't expect any convention to be followed, you just have to mark it as draft and ask for a architectural review). The worst case scenario is misinterpreting the specs and come back with something correct but different than was it requested. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[gpsaggese]

@aish-nidhi can you reply on the GUI (the link is in the email) instead of replying to the GH email so that we don't get all the garbage above.

[aish-nidhi]

@gpsaggese Will keep that in mind.

[aish-nidhi]

Hey @DanilYachmenev @samarth9008 ,

Just had a quick question. I was trying to run my test when I realised that both the files mentioned in the issue are not using the function calculate_vwap() from transform_utils.py. Instead they are actually using calculate_vwap_twap() function from transform.py.

Below is a jackpy search result output:-

(amp.client_venv) ➜  sorrentum1 git:(SorrTask782_Unit_test_calculate_vwap_function) ✗ jackpy "calculate_vwap"
research_amp/transform.py:20:def calculate_vwap_twap(df: pd.DataFrame, resampling_rule: str) -> pd.DataFrame:
research_amp/cc/notebooks/CMTask1704_Get_raw_CC_data.py:93:def calculate_vwap_twap(df: pd.DataFrame, resampling_rule: str) -> pd.DataFrame:
research_amp/cc/notebooks/CMTask1704_Get_raw_CC_data.py:293:vwap_twap_df = calculate_vwap_twap(data_hist_ccxt, resampling_rule)
research_amp/cc/notebooks/Master_Analysis_CrossSectionalLearning.py:139:df = ramptran.calculate_vwap_twap(
research_amp/cc/notebooks/master_tradability_analysis.py:122:vwap_twap_df = ramptran.calculate_vwap_twap(ohlcv_cc, resampling_rule)
im_v2/common/data/transform/test/test_transform_utils.py:803:    def test_calculate_vwap_with_resample_kwargs(self) -> None:
im_v2/common/data/transform/test/test_transform_utils.py:809:    def test_calculate_vwap_no_resample_kwargs(self) -> None:
im_v2/common/data/transform/transform_utils.py:374:def calculate_vwap(
./Untitled.py:42:vwap_twap_df = ramptran.calculate_vwap_twap(ohlcv_cc, resampling_rule)
(amp.client_venv) ➜  sorrentum1 git:(SorrTask782_Unit_test_calculate_vwap_function) ✗ 

Can you point me to somewhere this function is used? Should I continue with testing?

[DanilYachmenev]

@aish-nidhi good note, indeed, calculate_vwap() is not used in the lib
Instead, calculate_vwap_twap() is
And it needs to be tested too, so please add tests for it.

So here you need to create a test DataFrame with "full_symbol" (str, see examples in the codebase), "close" (float) and "volume" (float) columns.
Also use a pd.Timestamp index with 1-min freq, make like ~20 rows

then just add 2 tests for 2 different resampling_rule values

Use string signature approach in order to compare outputs.
See tests in helpers/test/test_hpandas.py for reference (and unit test doc ofc)

[aish-nidhi]

Hey @DanilYachmenev @samarth9008,
As mentioned by you, I put full_symbol as "str". But in the calculate_vwap_twap() function, there is a call to convert_to_multiindex() where full_symbol is being passed as assert_id_col and that function asserts if the column is np.int64.

How would you want me to address this issue? Should I convert the "full_symbol" to "np.int64"? The convert_to_multiindex() function is being used at too many places, so making alterations to it might first need an effect analysis of it.

[DanilYachmenev]

... in the calculate_vwap_twap() function, there is a call to convert_to_multiindex() where full_symbol is being passed as assert_id_col and that function asserts if the column is np.int64.

@aish-nidhi

Indeed, true. calculate_vwap_twap() calls convert_to_multiindex() which has been refactored to process input data with asset id columns which is reflected in unit tests for it
see Test_convert_to_multiindex, you can use approach to building test dataframe from it

For some reason, functions from amp/research_amp/transform.py were not updated to this behaviour, unpaid tech debt
usage of calculate_vwap_twap() for now is limited only by some obsolete notebooks but can be useful in research soon

Therefore, replace "full_symbol" column name to "asset_id" and create test dataframe with "asset_id" that has int values
then function will work properly and you can write unit tests

[samarth9008]

Not used anymore so closing