Add support for data download during backtest and refactor of catalog #2652
Conversation
faysou
force-pushed
the
download_data
branch
5 times, most recently
from
94bee45 to
d3ae70b
Compare
|
|
faysou
force-pushed
the
download_data
branch
11 times, most recently
from
2ac3ff8 to
8d5b794
Compare
faysou
force-pushed
the
download_data
branch
8 times, most recently
from
161fd38 to
1e28b9b
Compare
faysou
force-pushed
the
download_data
branch
4 times, most recently
from
d5b6a85 to
6f8617e
Compare
faysou
changed the title
faysou
force-pushed
the
download_data
branch
6 times, most recently
from
8083561 to
be93e2b
Compare
| if success_msg: | ||
| self._log.info(success_msg, success_color) | ||
| self._log.info(f"{success_msg} (Color: {success_color})") |
There was a problem hiding this comment.
Did we mean to change passing the LogColor enum here?
| kwargs : Any | ||
| Additional keyword arguments to be passed to the `write_chunk` method. | ||
| start : int, optional | ||
| The start timestamp for the data chunk. |
There was a problem hiding this comment.
Is it Unix nanos? (we should document that, param names are fine)
Also, should it be inferred from that data [0] and [-1] to prevent mismatches? (there might be a reason for this I haven't read yet)
[edit] Is it because we're carrying the original start and end metadata for the query which produced the data? we could probably provide a little more detail about the behavior in the docstring, which we'll carry over to Rust - because when I saw start and end I immediately thought of the potential for mismatches.
| self.fs.mkdirs(directory, exist_ok=True) | ||
|
|
||
| if isinstance(data[0], Instrument): | ||
| # When writing an instrument for a given instrument_id, we don't want duplicates |
There was a problem hiding this comment.
We'll eventually want to change this to handling a series of instrument versions, to reflect changes to the definition over time. (Understood that this preserves existing functionality)
| ) -> None: | ||
| """ | ||
| Consolidate several parquet files into a single file with data sorted in | ||
| ascending chronological order. | ||
| Reset the filenames of parquet files for a specific data class and instrument |
| @@ -34,14 +35,11 @@ def class_to_filename(cls: type) -> str: | |||
| return name | |||
|
|
|||
|
|
|||
| def urisafe_instrument_id(instrument_id: InstrumentId | str) -> str: | |||
| def urisafe_instrument_id(instrument_id: InstrumentId | BarType | str) -> str: | |||
There was a problem hiding this comment.
Should we choose a different param name if it could also be a bar type?
| @@ -107,6 +107,8 @@ cdef class SubscribeData(DataCommand): | |||
| ---------- | |||
| data_type : type | |||
| The data type for the subscription. | |||
| instrument_id : InstrumentId | |||
There was a problem hiding this comment.
I'm a little on the fence about making instrument_id optional and adding it to the more general subscribe custom data message. Does it make downstream logic in the catalog easier?
Pull Request
NautilusTrader prioritizes correctness and reliability, please follow existing patterns for validation and testing.
Summary
Add download function to backtest node in order to be able to download data and save it to a catalog easily.
This PR builds on previous PRs (see their comments for more details on each of them).
To reduce maintance, I will only rebase this PR while waiting it gets merged.
Related Issues/PRs
#2574
#2594
Type of change
Release notes
RELEASES.mdthat follows the existing conventions (when applicable)Testing
Tested on included example