Skip to content

Fix portfolio mode by adding secAccNo parameter to compactPortfolio#256

Open
maximilian-sh wants to merge 1 commit intopytr-org:masterfrom
maximilian-sh:fix/portfolio-secaccno-246
Open

Fix portfolio mode by adding secAccNo parameter to compactPortfolio#256
maximilian-sh wants to merge 1 commit intopytr-org:masterfrom
maximilian-sh:fix/portfolio-secaccno-246

Conversation

@maximilian-sh
Copy link

@maximilian-sh maximilian-sh commented Nov 6, 2025

Fixes #246

The Trade Republic API now requires secAccNo in the compactPortfolio subscription. Without it the API returns an empty portfolio silently.

Changes

  • Cache securitiesAccountNumber from settings() response into self._sec_acc_no
  • Pass secAccNo in compact_portfolio(), fetching it lazily if not yet available
  • Raise a clear error if the account number can't be retrieved

Testing

  • Confirmed v0.4.6 without fix returns empty portfolio
  • Confirmed this branch returns all positions correctly

pytr/api.py Outdated
_subscription_id_counter = 1
_previous_responses: Dict[str, str] = {}
subscriptions: Dict[str, Dict[str, Any]] = {}
secAccNo = None
Copy link
Contributor

Choose a reason for hiding this comment

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

I would make this private

Copy link
Author

Choose a reason for hiding this comment

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

Done, renamed to _secAccNo.

self.secAccNo = account["id"]
break
return settings_data

Copy link
Contributor

Choose a reason for hiding this comment

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

Are all these locations still relevant to try?

In my case the securities account number is stored in settings_data["securitiesAccountNumber"]

Copy link
Author

Choose a reason for hiding this comment

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

Good point — simplified to only use settings_data["securitiesAccountNumber"]. The other fallbacks were speculative. Same field works for me as well.

@maximilian-sh maximilian-sh force-pushed the fix/portfolio-secaccno-246 branch from cbfe507 to d43535b Compare February 1, 2026 13:29
@maximilian-sh maximilian-sh force-pushed the fix/portfolio-secaccno-246 branch from 408e7c4 to 96c5c15 Compare March 2, 2026 09:43
@maximilian-sh
Copy link
Author

Looked at the code again and reworked it a bit — moved _sec_acc_no to an instance attribute in __init__ instead of class level, dropped the unused compact_portfolio_by_type, and cleaned up the variable names. Logic is the same. Also rebased on master. Hope this can get merged.

@maximilian-sh maximilian-sh force-pushed the fix/portfolio-secaccno-246 branch from 96c5c15 to 60dda81 Compare March 2, 2026 09:46
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

Successfully merging this pull request may close these issues.

Portfolio mode not working

2 participants