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

Test compute_fill_stats() #1001

Open
samarth9008 opened this issue May 23, 2024 · 7 comments
Open

Test compute_fill_stats() #1001

samarth9008 opened this issue May 23, 2024 · 7 comments
Assignees

Comments

@samarth9008
Copy link
Collaborator

We want to add unit tests for compute_fill_stats()

Function location -

def compute_fill_stats(target_position_df: pd.DataFrame) -> pd.DataFrame:

TODO(*): How does the input target position DF looks like?

See unit test doc to follow the code style
Also highly recommend to read this doc before submitting 1st PR

FYI @gpsaggese @sonaalKant @DanilYachmenev

@samarth9008 samarth9008 self-assigned this May 23, 2024
@gpsaggese
Copy link
Contributor

@samarth9008 let's make the "See ..." as checklists so we force people to actually do it.

This is a great function to unit test BTW.

@DanilYachmenev
Copy link
Collaborator

@samarth9008 as to your TODO

input df should contain 2-level columns with a certain subset at 0 level and timestamp index + some raws to make reasonable computations

smth like this can work

data = {
    ("holdings_notional", 1): [1000, 1200, 1100, 1300, 1400],
    ("holdings_notional", 2): [800, 850, 870, 900, 950],
    ("holdings_shares", 1): [10, 12, 11, 13, 14],
    ("holdings_shares", 2): [8, 9, 8.5, 9.5, 10],
    ("price", 1): [100, 105, 102, 108, 110],
    ("price", 2): [50, 52, 51, 53, 54],
    ("target_holdings_notional", 1): [950, 1150, 1050, 1250, 1350],
    ("target_holdings_notional", 2): [780, 830, 850, 880, 930],
    ("target_holdings_shares", 1): [9, 11, 10, 12, 13],
    ("target_holdings_shares", 2): [7.5, 8.5, 8.2, 9.2, 9.5],
    ("target_trades_shares", 1): [1, 2, -1, 2, 1],
    ("target_trades_shares", 2): [0.5, 1, -0.3, 0.7, 0.5],
}

index = pd.date_range(start="2023-01-01", periods=5, freq="T")
columns = pd.MultiIndex.from_tuples(data.keys())
target_position_df = pd.DataFrame(data.values(), index=index, columns=columns)

@samarth9008 samarth9008 assigned surbhi498 and unassigned samarth9008 May 27, 2024
@samarth9008
Copy link
Collaborator Author

@surbhi498 Whats the update?

@surbhi498
Copy link
Contributor

@samarth9008 I am done writing test cases shortly raising the PR.
Thanks

@surbhi498
Copy link
Contributor

@samarth9008 PR for this issue has been raised.
Thanks

@gpsaggese
Copy link
Contributor

No need to thank after every msg. Can you pls post the link to the PR? The goal is to make the reader's life easier.

@surbhi498
Copy link
Contributor

#1012 (comment)

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

No branches or pull requests

4 participants