8000 Apply Commissions to PnL. by abhirathmahipal · Pull Request #1279 · kernc/backtesting.py · GitHub
[go: up one dir, main page]
Skip to content

Apply Commissions to PnL. #1279

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.

Sign up for GitHub

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

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

Apply Commissions to PnL. #1279

wants to merge 1 commit into from

Conversation

abhirathmahipal
Copy link
Contributor

No description provided.

@abhirathmahipal
Copy link
Contributor Author

Will take up pl_pct and commission calculation bugs in the next CR.

This should fix the PnL fields and other downstream calculations in compute_stats.

kernc
kernc reviewed May 28, 2025
View reviewed changes
@kernc
Copy link
Owner
kernc commented May 28, 2025

Why not apply the same subtraction (-self._commissions) to Trade.pl_pct though? I.e. to keep the two PL values in sync. 🟦

Not much should change other than some test values? 🤔

@abhirathmahipal
Copy link
Contributor Author
8000 abhirathmahipal commented May 29, 2025

Why not apply the same subtraction (-self._commissions) to Trade.pl_pct though? I.e. to keep the two PL values in sync. 🟦

This would work for Broker but not for Trade. If parity has to be maintained in Trade, we'll have to factor commission into per unit price.

@abhirathmahipal abhirathmahipal force-pushed the dev/account_for_commission branch from 56681cf to c163803 Compare May 29, 2025 01:32
@abhirathmahipal
Copy link
Contributor Author

Let's merge this?

As mentioned above

Will take up pl_pct and commission calculation bugs in the next CR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kernc kernc kernc left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@abhirathmahipal @kernc
0