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

SorrTask-782 Unit tests for calculate_vwap_twap() #801

Merged
merged 11 commits into from
May 3, 2024

Conversation

aish-nidhi
Copy link
Collaborator

Created a draft PR to illustrate the code skeleton.

@aish-nidhi
Copy link
Collaborator Author

@gpsaggese Please review the skeleton.

@samarth9008
Copy link
Collaborator

The skeleton looks good. Feel free to move forward with testing.

@aish-nidhi
Copy link
Collaborator Author

sure. Thanks for the update.

@aish-nidhi
Copy link
Collaborator Author

I wanted to understand the data being tested in the calculate_vwap function. I found the volume column but couldn’t find a price column. Also observing the code, led me to a resampling rule value of “5T” and just wanted to confirm if I’m on the right track for the resampling rule or is it something else?

@aish-nidhi
Copy link
Collaborator Author

aish-nidhi commented Apr 23, 2024

Pasted Graphic Pasted Graphic 1

Added both unit tests with different resample rules. Test case passing successfully.

@aish-nidhi
Copy link
Collaborator Author

Linter run is still pending.
Trying to resolve an environment issue due to which my linter is not working.

im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
im_v2/common/data/transform/test/test_transform_utils.py Outdated Show resolved Hide resolved
@DanilYachmenev
Copy link
Collaborator

@DanilYachmenev DanilYachmenev added the PR_for_authors The PR needs changes label Apr 25, 2024
@aish-nidhi aish-nidhi added PR_for_reviewers The PR needs to be reviewed by Team Leaders and removed PR_for_authors The PR needs changes labels Apr 26, 2024
@aish-nidhi
Copy link
Collaborator Author

Passed tests post changes.

image

research_amp/test/test_transform.py Outdated Show resolved Hide resolved
research_amp/test/test_transform.py Outdated Show resolved Hide resolved
research_amp/test/test_transform.py Outdated Show resolved Hide resolved
research_amp/test/test_transform.py Outdated Show resolved Hide resolved
@DanilYachmenev DanilYachmenev added PR_for_authors The PR needs changes and removed PR_for_reviewers The PR needs to be reviewed by Team Leaders labels Apr 27, 2024
@aish-nidhi aish-nidhi added PR_for_reviewers The PR needs to be reviewed by Team Leaders and removed PR_for_authors The PR needs changes labels Apr 29, 2024
@aish-nidhi aish-nidhi changed the title Issue #782 draft PR with code skeleton SorrTask-782 Unit tests for calculate_vwap_twap() Apr 29, 2024
Copy link
Collaborator

@DanilYachmenev DanilYachmenev left a comment

Choose a reason for hiding this comment

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

nice, just couple nits and ready

research_amp/test/test_transform.py Show resolved Hide resolved
research_amp/test/test_transform.py Show resolved Hide resolved
@DanilYachmenev DanilYachmenev added PR_for_authors The PR needs changes and removed PR_for_reviewers The PR needs to be reviewed by Team Leaders labels Apr 29, 2024
@aish-nidhi aish-nidhi added PR_for_reviewers The PR needs to be reviewed by Team Leaders and removed PR_for_authors The PR needs changes labels Apr 29, 2024
Copy link
Collaborator

@DanilYachmenev DanilYachmenev left a comment

Choose a reason for hiding this comment

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

@DanilYachmenev DanilYachmenev added PR_for_integrators The PR needs to be reviewed and possibly merged and removed PR_for_reviewers The PR needs to be reviewed by Team Leaders labels Apr 29, 2024
Copy link
Collaborator

@samarth9008 samarth9008 left a comment

Choose a reason for hiding this comment

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

LG

@samarth9008 samarth9008 merged commit 9be0264 into master May 3, 2024
1 check passed
@samarth9008 samarth9008 deleted the SorrTask782_Unit_test_calculate_vwap_function branch May 3, 2024 16:50
Kev-Daran pushed a commit to Kev-Daran/kaizenflow that referenced this pull request May 8, 2024
* Issue causify-ai#782 draft PR with code skeleton

* Issue causify-ai#782 unit tests for function calculate_vwap_twap with 2 different resampling rules

* Fix for ambiguous truth value of multiindex column names

* Linter corrections on file

* Changes for PR comment

* Modifications to address PR comments

* Adding full-stop to comments

---------

Co-authored-by: Aishwarya Nidhi <aishwaryanidhi@Aishwaryas-Air.cgocable.net>
Co-authored-by: Samarth KaPatel <samarth.kapatel5@gmail.com>
DIPOLAWSON pushed a commit that referenced this pull request May 9, 2024
* Issue #782 draft PR with code skeleton

* Issue #782 unit tests for function calculate_vwap_twap with 2 different resampling rules

* Fix for ambiguous truth value of multiindex column names

* Linter corrections on file

* Changes for PR comment

* Modifications to address PR comments

* Adding full-stop to comments

---------

Co-authored-by: Aishwarya Nidhi <aishwaryanidhi@Aishwaryas-Air.cgocable.net>
Co-authored-by: Samarth KaPatel <samarth.kapatel5@gmail.com>
Nadiyahlw added a commit that referenced this pull request May 9, 2024
* Revert "Add DATA605 directory (#876)" (#884)

This reverts commit 00e90d2.

* SorrTask786 Unit test for split_positive_and_negative_parts() (#883)

* Added Unit test for split_positive_and_negative_parts()

* Resolved comments

* Added remaining comments

* deleted unnecessary files

* "Fixes"

* Comment resolution

* Comments resolved.

---------

Co-authored-by: Samarth KaPatel <samarth.kapatel5@gmail.com>

* SorrTask-782 Unit tests for calculate_vwap_twap() (#801)

* Issue #782 draft PR with code skeleton

* Issue #782 unit tests for function calculate_vwap_twap with 2 different resampling rules

* Fix for ambiguous truth value of multiindex column names

* Linter corrections on file

* Changes for PR comment

* Modifications to address PR comments

* Adding full-stop to comments

---------

Co-authored-by: Aishwarya Nidhi <aishwaryanidhi@Aishwaryas-Air.cgocable.net>
Co-authored-by: Samarth KaPatel <samarth.kapatel5@gmail.com>

* SorrTask-891 Unit tests for compute epoch function (#901)

* Issue 891 - Unit test code skeleton

* Issue-891 3 unit tests with each available unit value for Series input.

* Issue-891 Unit test of compute epoch function for dataframe input with default unit

* Adding missed full stop for comment

* PR comment fixes

* Removing todo from function

* PR comment fixes

---------

Co-authored-by: Aishwarya Nidhi <aishwaryanidhi@Aishwaryas-Air.cgocable.net>
Co-authored-by: Samarth KaPatel <samarth.kapatel5@gmail.com>

* SorTask903 Unit test convert_seconds_to_pandas_minutes()  (#907)

* Added unit test for function convert_seconds_to_minutes

* Nits

---------

Co-authored-by: Shaunak  Dhande <77265046+Shaunak01@users.noreply.github.com>
Co-authored-by: neha2801-create <77967216+neha2801-create@users.noreply.github.com>
Co-authored-by: Samarth KaPatel <samarth.kapatel5@gmail.com>
Co-authored-by: Aishwarya Nidhi <nidhiaishwarya8@gmail.com>
Co-authored-by: Aishwarya Nidhi <aishwaryanidhi@Aishwaryas-Air.cgocable.net>
@Nadiyahlw Nadiyahlw mentioned this pull request May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR_for_integrators The PR needs to be reviewed and possibly merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants