-
Notifications
You must be signed in to change notification settings - Fork 190
feat: Add auction data handling to stock historical data client #576
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
base: master
Are you sure you want to change the base?
Conversation
- Introduced `StockAuctionsRequest` for auction data requests. - Implemented `get_stock_auction` method in `StockHistoricalDataClient` to retrieve auction data. - Added `Auction` and `AuctionSet` models for structured auction data representation. - Updated mappings to include auction-related fields. - Created example usage for fetching auction data in `examples/stocks/test.py`.
- Deleted `examples/stocks/test.py` which contained example usage for fetching auction data using the `StockHistoricalDataClient`. - This cleanup is part of the ongoing effort to streamline example files and focus on essential components.
…tting - Added missing commas in the function signatures of `validate_symbol_or_asset_id` and `validate_symbol_or_contract_id` for consistency. - Improved code formatting in `BaseTimeseriesDataRequest`, `StockHistoricalDataClient`, and `Auction` classes by removing unnecessary blank lines and ensuring proper spacing. - Enhanced readability and maintainability of the codebase.
I ve reformatted the file using black. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR! Overall LGTM! Left some comments. Could you please consider them?
alpaca/data/historical/stock.py
Outdated
@@ -72,6 +73,25 @@ def __init__( | |||
raw_data=raw_data, | |||
) | |||
|
|||
def get_stock_auction(self, request_params: StockAuctionsRequest): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use plural?
def get_stock_auction(self, request_params: StockAuctionsRequest): | |
def get_stock_auctions(self, request_params: StockAuctionsRequest): |
# ############################## Auctions ################################# # | ||
|
||
|
||
class StockAuctionsRequest(BaseTimeseriesDataRequest): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please write docstring and also add to api_reference?
alpaca/data/models/auctions.py
Outdated
for auction in auctions: | ||
c = auction.get("c") | ||
o = auction.get("o") | ||
|
||
if c is not None: | ||
auction_data.extend(c) | ||
if o is not None: | ||
auction_data.extend(o) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discussion: I felt it might be good to have indicator or separation between open and close. even though it might be possible to know based on timestamp. prefer to put something easy to distinguish.
idea: how about adding another attribuite i.e. open or close?
"""Represents one auction of aggregated trade data over a specified interval. | ||
|
||
Attributes: | ||
symbol (str): The ticker identifier for the security whose data forms the bar. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you please add other attribuites to docstring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all done
- Added `auction_type` field to the `Auction` model for better auction data representation. - Updated `StockAuctionsRequest` with detailed docstring for clarity on parameters. - Renamed `get_stock_auction` method to `get_stock_auctions` for consistency with plural naming conventions. - Updated mappings to include the new `auction_type` field. - Enhanced API documentation for auction-related models and requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM! sorry, left one more comment. could you please address the comment?
alpaca/data/models/auctions.py
Outdated
if o is not None: | ||
for open_auction in o: | ||
if open_auction: | ||
open_auction["at"] = "OPEN" | ||
auction_data.extend(o) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about putting open
first before close
as timestamp could be orderd by asc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use orderby timestamp in the final dataframe, and then open will be before close.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. You are right. For the dataframe, it is easy. But also we have AuctionSet object and if we change here, the process not required in dataframe level as well? Do you have any concern to change order here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying that the order of the AuctionSet should be consistent with the order of the dataframe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I thought is order by open
=> close
would be nicer than close
=> open
order in a day.
currently:
{
"data": {
"SPY": [
{
"auction_type": "CLOSE",
...
"timestamp": datetime.datetime(
2025, 4, 2, 20, 0, 0, 42680, tzinfo=TzInfo(UTC)
),
},
...
{
"auction_type": "OPEN",
...
"timestamp": datetime.datetime(
2025, 4, 2, 13, 30, 0, 330621, tzinfo=TzInfo(UTC)
),
},
...
{
"auction_type": "CLOSE",
...
"timestamp": datetime.datetime(
2025, 4, 3, 20, 0, 0, 130854, tzinfo=TzInfo(UTC)
),
},
...
{
"auction_type": "OPEN",
...
"timestamp": datetime.datetime(
2025, 4, 3, 13, 30, 0, 692296, tzinfo=TzInfo(UTC)
),
},
...
]
}
}
What I thought good is:
{
"data": {
"SPY": [
{
"auction_type": "OPEN",
...
"timestamp": datetime.datetime(
2025, 4, 2, 13, 30, 0, 330621, tzinfo=TzInfo(UTC)
),
},
...
{
"auction_type": "CLOSE",
...
"timestamp": datetime.datetime(
2025, 4, 2, 20, 0, 0, 42680, tzinfo=TzInfo(UTC)
),
},
...
{
"auction_type": "OPEN",
...
"timestamp": datetime.datetime(
2025, 4, 3, 13, 30, 0, 692296, tzinfo=TzInfo(UTC)
),
},
...
{
"auction_type": "CLOSE",
...
"timestamp": datetime.datetime(
2025, 4, 3, 20, 0, 0, 130854, tzinfo=TzInfo(UTC)
),
},
...
]
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it
- Updated the `AuctionSet` model to sort auction data by timestamp in ascending order before parsing. - Improved data integrity and consistency for auction data representation.
alpaca/data/models/auctions.py
Outdated
@@ -86,10 +86,16 @@ def __init__(self, raw_data: RawData) -> None: | |||
open_auction["at"] = "OPEN" | |||
auction_data.extend(o) | |||
|
|||
parsed_auctions[symbol] = [ | |||
auction_data = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a sorted func to solve the issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. What I thought was we already specifying order in the parameter. Therefore, just need to adjust the order of extend. but we realized we might want to adjust based on request.sort?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed with the owner of the API endpoint and seems the sort parameter considering timestamp of each auction price. Therefore, I feel if asc, then extend open first. if desc, then extend close first might be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean adding a parameter asc or desc? My thought is that we could let the user decide ascending or descending, so we can add a request parameter ascending: bool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant sorting order should be aligned with users input (i.e. sort
parameter of request (ref. default to asc)).
Current PR has sorting by ASC without considering users input.
The other point is that the response of API is already sorted by symbol and timestamp based on users input. Therefore, we could utilize response's sorted order by changing order of extend based on user's input?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it
- Modified the `AuctionSet` constructor to accept a `sort` parameter for auction data ordering. - Implemented conditional logic to sort auction data based on the specified sort order (ascending or descending). - Enhanced data integrity by ensuring auction data is parsed and ordered correctly before being stored.
if sort == Sort.DESC: | ||
if c is not None: | ||
for close_auction in c: | ||
if close_auction: | ||
close_auction["at"] = "CLOSE" | ||
auction_data.extend(c) | ||
if o is not None: | ||
for open_auction in o: | ||
if open_auction: | ||
open_auction["at"] = "OPEN" | ||
auction_data.extend(o) | ||
else: # NOTE: None and ASC are the same | ||
if o is not None: | ||
for open_auction in o: | ||
if open_auction: | ||
open_auction["at"] = "OPEN" | ||
auction_data.extend(o) | ||
if c is not None: | ||
for close_auction in c: | ||
if close_auction: | ||
close_auction["at"] = "CLOSE" | ||
auction_data.extend(c) | ||
|
||
parsed_auctions[symbol] = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the sorting func and ustilize the asc/desc in requesting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! By the way, what is the meaning of if open_auction
/if close_auction
? Do you have observed something we need to handle the empty element? I felt we use for
in the code, the element should have value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM! Left small comments.
|
||
data: Dict[str, List[Auction]] = {} | ||
|
||
def __init__(self, raw_data: RawData, sort: Sort) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StockAuctionsRequest.sort is Optional[Sort]
def __init__(self, raw_data: RawData, sort: Sort) -> None: | |
def __init__(self, raw_data: RawData, sort: Optional[Sort]) -> None: |
@@ -0,0 +1,108 @@ | |||
from datetime import datetime | |||
from typing import Dict, List |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to use Optional
from typing import Dict, List | |
from typing import Dict, List, Optional |
|
||
Args: | ||
raw_data (RawData): The collection of raw auction data from API keyed by Symbol. | ||
sort (Sort): The sort order of the auctions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sort (Sort): The sort order of the auctions. | |
sort (Optional[Sort]): The sort order of the auctions. Default to ASC |
if sort == Sort.DESC: | ||
if c is not None: | ||
for close_auction in c: | ||
if close_auction: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we remove this if condition? or do we have possiblity to have None element in the list?
c = auction.get("c") | ||
o = auction.get("o") | ||
|
||
if sort == Sort.DESC: | ||
if c is not None: | ||
for close_auction in c: | ||
if close_auction: | ||
close_auction["at"] = "CLOSE" | ||
auction_data.extend(c) | ||
if o is not None: | ||
for open_auction in o: | ||
if open_auction: | ||
open_auction["at"] = "OPEN" | ||
auction_data.extend(o) | ||
else: # NOTE: None and ASC are the same | ||
if o is not None: | ||
for open_auction in o: | ||
if open_auction: | ||
open_auction["at"] = "OPEN" | ||
auction_data.extend(o) | ||
if c is not None: | ||
for close_auction in c: | ||
if close_auction: | ||
close_auction["at"] = "CLOSE" | ||
auction_data.extend(c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about below logic? tried to use .get("c", [])
but it does not work for me.
c = auction.get("c") | |
o = auction.get("o") | |
if sort == Sort.DESC: | |
if c is not None: | |
for close_auction in c: | |
if close_auction: | |
close_auction["at"] = "CLOSE" | |
auction_data.extend(c) | |
if o is not None: | |
for open_auction in o: | |
if open_auction: | |
open_auction["at"] = "OPEN" | |
auction_data.extend(o) | |
else: # NOTE: None and ASC are the same | |
if o is not None: | |
for open_auction in o: | |
if open_auction: | |
open_auction["at"] = "OPEN" | |
auction_data.extend(o) | |
if c is not None: | |
for close_auction in c: | |
if close_auction: | |
close_auction["at"] = "CLOSE" | |
auction_data.extend(c) | |
c = auction.get("c") | |
if c is None: | |
c = [] | |
o = auction.get("o") | |
if o is None: | |
o = [] | |
for close_auction in c: | |
close_auction["at"] = "CLOSE" | |
for open_auction in o: | |
open_auction["at"] = "OPEN" | |
if sort == Sort.DESC: | |
auction_data.extend(c) | |
auction_data.extend(o) | |
else: # NOTE: None and ASC are the same | |
auction_data.extend(o) | |
auction_data.extend(c) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.get
I think both are fine. c = [] if c is None else c
or .get("c", [])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Yeah, somehow, .get("c", [])
does not work for me. checking with below snippets.
from datetime import datetime
from zoneinfo import ZoneInfo
from alpaca.common.enums import Sort
from alpaca.data.historical.stock import StockAuctionsRequest, StockHistoricalDataClient
api_key="API KEY"
secret_key="SECRET"
client = StockHistoricalDataClient(api_key, secret_key)
req = StockAuctionsRequest(
symbol_or_symbols=["SPY"],
start=datetime(2025, 4,2),
end=datetime(2025, 4, 9, tzinfo=ZoneInfo("America/New_York")),
sort=Sort.DESC,
)
ret = client.get_stock_auctions(req)
raw_auctions = self._get_marketdata( | ||
path="/stocks/auctions", | ||
params=request_params.to_request_fields(), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have recently introduced page_size
parameter to _get_marketdata()
[1], could you please set page_size for auctions as well?
*1 #583
raw_auctions = self._get_marketdata( | |
path="/stocks/auctions", | |
params=request_params.to_request_fields(), | |
) | |
raw_auctions = self._get_marketdata( | |
path="/stocks/auctions", | |
params=request_params.to_request_fields(), | |
page_limit = 1_000, | |
page_size = 1_000, | |
) |
StockAuctionsRequest
for auction data requests.get_stock_auction
method inStockHistoricalDataClient
to retrieve auction data.Auction
andAuctionSet
models for structured auction data representation.examples/stocks/test.py
.