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

Implement mixed catalog data requests with catalog update #2043

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

faysou
Copy link
Collaborator

@faysou faysou commented Nov 5, 2024

Pull Request

Add update_catalog method to actor class

Type of change

  • New feature (non-breaking change which adds functionality)

How has this change been tested?

Added several tests

@faysou faysou closed this Nov 9, 2024
@faysou faysou reopened this Nov 9, 2024
@faysou faysou force-pushed the update_catalog branch 2 times, most recently from 7497951 to 069dc2a Compare November 11, 2024 09:41
@faysou faysou changed the title Add update_catalog method to actor class Allow mixed catalog / market data requests and add update_catalog optional argument Nov 11, 2024
Copy link

codspeed-hq bot commented Nov 11, 2024

CodSpeed Performance Report

Merging #2043 will not alter performance

Comparing faysou:update_catalog (819e2ed) with develop (dda454b)

Summary

✅ 52 untouched benchmarks

@faysou faysou force-pushed the update_catalog branch 2 times, most recently from da169f9 to 81f7701 Compare November 13, 2024 15:04
@faysou faysou marked this pull request as ready for review November 24, 2024 22:09
Copy link
Member

@cjdsellers cjdsellers left a comment

Choose a reason for hiding this comment

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

Awaiting some additional testing and tweaks before merging.

if start is not None:
Condition.is_true(start <= now, "start was > now")
if end is not None:
Condition.is_true(end <= now, "start was > now")
Copy link
Member

Choose a reason for hiding this comment

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

end

if start is not None:
Condition.is_true(start <= now, "start was > now")
if end is not None:
Condition.is_true(end <= now, "start was > now")
Copy link
Member

Choose a reason for hiding this comment

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

end

@@ -2142,6 +2156,8 @@ cdef class Actor(Component):
datetime end = None,
ClientId client_id = None,
callback: Callable[[UUID4], None] | None = None,
bint update_catalog = False,
str quote_type = "",
Copy link
Member

Choose a reason for hiding this comment

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

Is this to specify the bbo schemas?

I think this is fine, probably one of the only ways to do it without adding specific data types, which might be overkill.

if start is not None:
Condition.is_true(start <= now, "start was > now")
if end is not None:
Condition.is_true(end <= now, "start was > now")
Copy link
Member

Choose a reason for hiding this comment

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

end

if start is not None:
Condition.is_true(start <= now, "start was > now")
if end is not None:
Condition.is_true(end <= now, "start was > now")
Copy link
Member

Choose a reason for hiding this comment

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

end

if start is not None:
Condition.is_true(start <= now, "start was > now")
if end is not None:
Condition.is_true(end <= now, "start was > now")
Copy link
Member

Choose a reason for hiding this comment

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

end

@@ -1804,7 +1804,7 @@ def test_update_timer_with_test_clock_sends_single_bar_to_handler(self):
bid_size=Quantity.from_int(1),
ask_size=Quantity.from_int(1),
ts_event=1 * 60 * 1_000_000_000, # 1 minute in nanoseconds
ts_init=1 * 60 * 1_000_000_000, # 1 minute in nanoseconds
ts_init=1 * 60 * 1_000_000_000,
Copy link
Member

@cjdsellers cjdsellers Nov 25, 2024

Choose a reason for hiding this comment

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

There's a constant we might be able to use here, but I think it's hard to access from Python because of Cython.

@cjdsellers cjdsellers changed the title Allow mixed catalog / market data requests and add update_catalog optional argument Implement mixed catalog data requests with catalog update Nov 27, 2024
Copy link
Member

@cjdsellers cjdsellers left a comment

Choose a reason for hiding this comment

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

Great work here, also some nice tidy ups 👌.

@cjdsellers cjdsellers merged commit a84aeab into nautechsystems:develop Nov 27, 2024
11 checks passed
@faysou faysou deleted the update_catalog branch December 17, 2024 09:36
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