Skip to content

Conversation

faysou
Copy link
Collaborator

@faysou faysou commented Aug 20, 2025

Pull Request

NautilusTrader prioritizes correctness and reliability, please follow existing patterns for validation and testing.

  • I have reviewed the CONTRIBUTING.md and followed the established practices

Summary

  • Improve bar subscription by subscribing to enough bars like before to active the request of historical data, but filtering data we don't need. The start params parameter allows to configure the start of a subscription.
  • Improve the caching of the instrument provider

Related Issues/PRs

Type of change

  • Bug fix (non-breaking)
  • New feature (non-breaking)
  • Breaking change (impacts existing behavior)
  • Documentation update
  • Maintenance / chore

Breaking change details (if applicable)

Documentation

  • Documentation changes follow the style guide (docs/developer_guide/docs.md)

Release notes

  • I added a concise entry to RELEASES.md that follows the existing conventions (when applicable)

Testing

Ensure new or changed logic is covered by tests.

  • Affected code paths are already covered by the test suite
  • I added/updated tests to cover new or changed logic

@faysou faysou changed the title ib-bar-fix Refine subscribe_historical_bars in IB adapter Aug 20, 2025
@faysou faysou force-pushed the ib-bar-fix branch 3 times, most recently from 49d780b to 444f1df Compare August 21, 2025 12:37
@faysou faysou marked this pull request as draft August 21, 2025 20:37
@faysou faysou force-pushed the ib-bar-fix branch 2 times, most recently from 7904d34 to 8e42ad6 Compare August 22, 2025 14:33
@faysou faysou marked this pull request as ready for review August 22, 2025 14:33
@faysou faysou marked this pull request as draft August 22, 2025 19:08
@cjdsellers cjdsellers marked this pull request as ready for review August 22, 2025 22:44
Indicates whether revised bars should be handled or not.
historical : bool | None, optional
Indicates whether the bar data is historical. Defaults to False.
start: int, optional
Copy link
Member

Choose a reason for hiding this comment

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

I think if start is not a pd.Timestamp then we should always suffix the units, like start_ns. The choice to use nanos depends on the abstraction level, generally passing around a pd.Timestamp is a little nicer unless it's passing into Cython territory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Passing timestamp in messages can pose serialisation issues. I can refactor to rename start as starts_ns.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's a good example of when we'd want to use a nanoseconds integer.

@cjdsellers cjdsellers merged commit bb59586 into develop Aug 22, 2025
13 checks passed
@cjdsellers cjdsellers deleted the ib-bar-fix branch August 22, 2025 23:42
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.

2 participants