[jhonabreul]

Description

End-of-run persistence to the ObjectStore is a common pattern, and its most frequent failures surface only in OnEndOfAlgorithm, losing the output of an otherwise green run:

LocalObjectStore: path is not supported: 'trade_log_ai_hardware_&_cloud.csv'
Object of type datetime is not JSON serializable
'ObjectStore' object has no attribute 'save_text' Did you mean: 'save'?

The changes:

• ObjectStore.IsSupportedKey / ValidateKey / SanitizeKey (static): key validation moves to the ObjectStore wrapper as the single source. ValidateKey rejects a key eagerly, SanitizeKey converts an arbitrary name into a supported key. The mapping is lossy and documented as such: collision checks stay with the caller.
• Rejection errors name the actual problem in the key (unsupported character '&', it has 2 '.' but at most one '.extension' is allowed, a non-alphanumeric extension), then the full key rules and point to SanitizeKey.
• Reading a missing key lists the available keys (first 10), so typos are visible in the error.
• SaveJson(path, PyObject) / ReadJson(path): Python JSON helpers for a dict, list, tuple or .NET object. Nested datetime/date, pandas Timestamp and numpy datetime64 as ISO-8601, durations as ISO-8601 durations, Decimal/numpy as numbers, NaN/infinity/NaT as null, .NET objects (Symbol included) through Newtonsoft like the C# SaveJson<T>. pandas objects and any type that would not round trip raise TypeError; top-level Python scalars are not accepted. The generic C# overloads are untouched.
• SaveDataframe(path, dataFrame) / ReadDataframe(path): pandas DataFrame/Series as CSV (JSON for .json keys). The read restores the frame, time index included; a Series comes back as a one-column DataFrame.
• SaveText / ReadText: aliases of Save(path, text, encoding) / Read(path, encoding).

Related Issue

N/A

Motivation and Context

Completed backtests can die on their final persistence call. Eager, self-explanatory validation plus faithful JSON turns these total-loss failures into first-try successes or loud, named errors.

Requires Documentation Change

Yes: the new ObjectStore members and the improved errors.

How Has This Been Tested?

• LocalObjectStoreTests: key validation, per-issue rejection hints, SanitizeKey cases and the collision case, missing-key listing, SaveText/ReadText, plus one Python test per save_json/read_json case (.NET objects, numpy, numpy datetimes and NaT, durations, NaN, NaN keys rejected, pandas rejected, unsupported types rejected, top-level scalars rejected), save_text/read_text, save_dataframe/read_dataframe and sanitize_key. Full storage suite: 162 passed, 0 failed.
• ObjectStoreErgonomicsRegressionAlgorithm (C# + Python): rejection hint, eager ValidateKey, sanitize-then-save, text round trip, missing-key error; the Python twin adds the JSON round trip with a Symbol, a .NET list, a numpy array with NaN, a datetime64 array and a rejected pandas Series, plus the DataFrame round trip. Both pass.
• The Python regression algorithm also ran end to end through the Launcher: every on_end_of_algorithm call succeeded, no runtime error.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• Refactor (non-breaking change which improves implementation)
• Performance (non-breaking change which improves performance. Please add associated performance test and results)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Non-functional change (xml comments/documentation/etc)

Checklist:

• My code follows the code style of this project.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• My branch follows the naming convention bug-<issue#>-<description> or feature-<issue#>-<description>

[arhancanli]

Read the diff (didn't run the suite). The regression algorithm's own expectation shows how SanitizeKey behaves: trade_log_ai_hardware_&_cloud.csv becomes trade_log_ai_hardware___cloud.csv, i.e. each unsupported character maps to a single _. That mapping is lossy and many-to-one: a&b.csv, a b.csv and an existing a_b.csv all sanitize to the same key, so two distinct names written in one run silently overwrite each other, the same kind of lost output this PR is meant to prevent. Worth a test that sanitizes two different names to the same key, and either documenting that SanitizeKey is not injective or offering a collision-safe variant (e.g. a short hash suffix when characters were replaced).

Smaller: SaveJson's fallback of str() for other types (Symbol and so on) is read back by ReadJson as a plain string, so a round trip changes the type; the PR text says this for the encoder, but the Python test would be clearer if it asserted what comes back for a Symbol, not just that saving succeeds.

[quantconnect-kronos-reviewer]

Requesting changes: one blocking defect in the new Python save_json, plus missing evidence.

CI is green on e956172. I built the branch and called the new Python methods through a local test against LocalObjectStore, then made the same calls on master to compare.

Blocking: save_json(path, obj) reports success but saves the wrong data for common values. On master, a Python save_json(key, x) without a type argument always raises TypeError (no overload matches), so the user knows the save failed. On this branch the call goes to the new PyObject overload. Any value the encoder doesn't recognize is saved as str(value), and the call returns True. What gets saved now:

value passed	saved JSON
List[int] [1, 2] (a .NET list)	"System.Collections.Generic.List`1[System.Int32]"
Dictionary[str, float]	"System.Collections.Generic.Dictionary`2[System.String,System.Double]"
IndicatorDataPoint(...)	"2020-01-01T00:00:00 - 5"
{'w': np.array([1.5, 2.5])}	{"w": "[1.5 2.5]"}
{'s': pd.Series([1, 2])}	{"s": "0 1\n1 2\ndtype: int64"}
{'w': float('nan')}	{"w": NaN} (not valid JSON; read_json in Python accepts it, strict JSON readers don't)

A run can now finish green and the user only finds out later, when they read the data back, that the output is gone. Losing that output is what this PR is meant to prevent. Suggested fix:

• Convert the common types: numpy.ndarray with .tolist(), and pandas objects with to_dict(), or reject them and point to save_dataframe.
• Send .NET objects to the existing Newtonsoft path, so List, Dictionary and Lean types serialize the way the C# SaveJson<T> does.
• Keep str() only for the types you list on purpose (Symbol). Anything else raises a TypeError that names the type.
• Add a Python test for each of the cases above.

An explicit generic call, save_json[List[int]](key, obj), still goes to the generic method and serializes correctly (checked), so existing typed calls are not affected.

Earlier comment not yet addressed: I agree with the open point that SanitizeKey maps many names to one key: a&b.csv, a b.csv and a_b.csv all become a_b.csv, so two names written in the same run silently overwrite each other. At least document this in the XML docs and add a test for it. A variant that can't collide, such as a short hash suffix added when characters were replaced, would be better.

Smaller points (non-blocking):

• save_json(key, "text") raises TypeError: No method matches, because a Python str doesn't bind to PyObject here. Either support it or leave it out of what save_json claims to accept.
• save_dataframe accepts a Series, but read_dataframe returns a DataFrame, so a round trip changes the type. Document this or handle it.
• GetKeyIssues has a single call site. The repository standard is to inline single-use methods.
• When a read misses, the error lists the keys by calling Keys, which walks the store's entries. That only happens on the error path, so it's fine. Code that probes keys with try/except should use ContainsKey, as the message already says.

Evidence: the unit tests and the regression algorithm cover the expected paths only, which is how the case above slipped through. This changes what Lean's algorithm API does for Python users. Please also run a Python algorithm end to end, the way users run it, that uses save_json/read_json, save_dataframe/read_dataframe, save_text and sanitize_key in on_end_of_algorithm. Include values from the table above, then add the outcome to the description.

Other repositories: the Python stubs are generated, but the documentation needs the new ObjectStore members and the new error messages, as the description says.

Risk label: risk:high. This PR adds members to Lean's user-facing algorithm API (SaveJson/ReadJson for Python objects, SaveDataframe/ReadDataframe, SaveText/ReadText, IsSupportedKey/ValidateKey/SanitizeKey), and changes the messages of existing errors. Additions to the algorithm API are high risk however they're tested.

[quantconnect-kronos-reviewer]

This fallback stores .NET objects as their type name (for example "System.Collections.Generic.List`1[System.Int32]"), a numpy array as "[1.5 2.5]" and a pandas Series as its printed text, and save_json still returns True. On master these calls raised TypeError. Please convert the common types, send .NET objects to Newtonsoft, keep str() only for the types listed on purpose (Symbol), and raise a TypeError that names the type for anything else.

[quantconnect-kronos-reviewer]

This mapping is many-to-one: a&b.csv, a b.csv and a_b.csv all become a_b.csv, so two names saved in one run overwrite each other without any error. Please document this here and add a test for it, or offer a variant that can't collide.

[jhonabreul]

Thanks, each point reproduced with a failing test first, then fixed.

save_json fallback (blocking). str() is gone. _normalize is now an allow-list: datetime/date/time as ISO-8601, Decimal and numpy scalars as numbers, numpy arrays via tolist(), NaN and infinity as null, .NET objects (Symbol included) through Newtonsoft so they match the C# SaveJson<T>, non-string keys as strings. pandas objects raise TypeError pointing to save_dataframe; any other type raises TypeError naming it. One Python unit test per row of your table, all red before the change.

SanitizeKey collisions. Documented as lossy in the XML docs, with SanitizeKeyIsLossyAndDistinctNamesCanCollide showing two names mapping to one key and the second save overwriting the first. No hash suffix: it only sanitizes, collision checks stay with the caller. One correction: a b.csv is a valid key (space is allowed) and is returned unchanged.

save_json(key, "text"). The binder rejects a Python primitive for a PyObject parameter when overloads exist, a pythonnet matter. For this PR the docs narrow the claim: dict, list, tuple or .NET object; top-level scalars raise and save_text is the way to store text. A test pins the rejection.

Series round trip. Documented on ReadDataframe: a saved Series comes back as a one-column DataFrame, squeeze() gives the Series.

GetKeyIssues. Kept as a private helper. It is ~40 lines of diagnosis with one three-line caller; inlining it would make ValidateKey the longest method in the class.

Evidence. The Python regression algorithm now includes a .NET list, a numpy array with NaN, a Symbol value and key, and a rejected pandas Series, and ran through the Launcher to completion with no runtime error. Full storage suite: 159 passed.

[quantconnect-kronos-reviewer]

Requesting changes for one remaining gap in the save_json allow-list. Everything else from my last review is resolved.

CI is green on 2405d2c. I built the branch and ran the storage tests locally: 173 passed. I also ran a throwaway probe test against LocalObjectStore for the edge cases below.

Resolved since my last review:

• The str() fallback is gone. .NET objects go through Newtonsoft, numpy arrays through tolist(), NaN and infinity become null, pandas objects raise and point to save_dataframe, and any other type raises a TypeError that names it. Each row of my earlier table has a test.
• SanitizeKey is documented as lossy, and a test shows the collision. You're right that a b.csv is a valid key (space is in the allowed set), so my earlier example was wrong about that one.
• The top-level scalar rejection, the Series round trip and the GetKeyIssues reasoning are all fine with me.
• The Python regression algorithm now covers the new cases, and the description reports an end-to-end run with no runtime error.

Blocking: numpy datetime values still save as something other than what was passed, and the call returns True. numpy.datetime64 is a numpy.generic, so .item() / .tolist() turns a nanosecond-precision value into an integer. pandas uses nanosecond precision by default. What my probe saved:

value passed	saved JSON
{'t': pd.to_datetime(['2020-01-01']).values} (e.g. history.index.values)	{"t": [1577836800000000000]}
{'t': np.datetime64('2020-01-01T00:00:00.000000000')}	{"t": 1577836800000000000}
{'t': pd.NaT}	{"t": "NaT"}

A Python datetime and a pd.Timestamp both save as ISO-8601, as documented. A numpy datetime64 should do the same, or be rejected, so it doesn't save as an integer. That keeps the docstring's promise that any type that would not round trip is rejected. Suggested fix: convert numpy.datetime64 values and datetime64 arrays to ISO-8601, for example through pandas.Timestamp(v).isoformat(), or with numpy.datetime_as_string for arrays. Save NaT as null, the same as NaN. Either convert numpy.timedelta64 or reject it, the way a Python timedelta is rejected today. Please add a test for each case.

Smaller points (non-blocking):

• A NaN dictionary key raises ValueError: Out of range float values are not JSON compliant from dumps, not the TypeError you use everywhere else. This is a rare case, so I'm only noting it.
• A NaN inside a .NET object is written as the string "NaN", which is Newtonsoft's default, while a Python NaN becomes null. That matches the documentation (".NET objects as the C# SaveJson<T> overload writes them"), so it's fine, but it may be worth one line in the user docs.

Other repositories: the Python stubs are generated. The documentation still needs the new ObjectStore members and error messages, as the description says.

Risk label: risk:high, unchanged. This PR adds members to Lean's user-facing algorithm API (SaveJson/ReadJson for Python objects, SaveDataframe/ReadDataframe, SaveText/ReadText, IsSupportedKey/ValidateKey/SanitizeKey) and changes the messages of existing errors. Additions to the algorithm API are high risk however they're tested.

[quantconnect-kronos-reviewer]

numpy.datetime64 is a numpy.generic, so a nanosecond value (the pandas default) comes back from .item() as an integer: np.datetime64('2020-01-01T00:00:00.000000000') saves as 1577836800000000000. The ndarray branch above has the same problem through tolist(), so history.index.values saves as a list of integers. pd.NaT saves as "NaT". Please convert datetime64 to ISO-8601 and NaT to null, and either convert or reject timedelta64, before falling through to .item() / .tolist().

[jhonabreul]

Thanks, reproduced each case with a failing test first.

numpy datetime64 / NaT (blocking). datetime64 scalars now save as ISO-8601 through pandas.Timestamp(v).isoformat(), nanoseconds kept; datetime64/timedelta64 arrays are converted element-wise instead of through tolist(), so history.index.values saves as ISO strings; pd.NaT and datetime64('NaT') save as null, like NaN. Python timedelta, pd.Timedelta and timedelta64 now save as ISO-8601 durations (P1DT2H0M0S), which pd.Timedelta parses back; timedelta64('NaT') is null. Tests: PythonSaveJsonWritesNumpyDatetimesAsIso8601AndNatAsNull and PythonSaveJsonWritesTimedeltasAsIso8601Durations, one assertion per case in your table plus a datetime64[D] grid and a pd.Timestamp. The Python regression algorithm also stores a DatetimeIndex.values now.

NaN dictionary key. Raises TypeError: save_json() cannot use nan as a dictionary key like the other rejections. Test: PythonSaveJsonRejectsNanDictionaryKeysWithTypeError.

NaN inside .NET objects. Added to the SaveJson docs: Newtonsoft writes it as the "NaN" string.

Storage suite: 162 passed. Both regression algorithms pass, and the Python one ran through the Launcher again to completion with no runtime error.

[quantconnect-kronos-reviewer]

Approving. The numpy datetime blocker from my last review is fixed, and the smaller points are handled too.

CI is green on f44c2fc. I built the branch and ran the storage tests locally: 172 passed. I also ran a throwaway probe test against LocalObjectStore for edge cases beyond the new tests.

Resolved since my last review:

• numpy.datetime64 scalars and datetime64/timedelta64 arrays now save as ISO-8601, nanoseconds included. Before, they saved as integers. history.index.values now saves as ISO strings.
• pd.NaT and datetime64('NaT') save as null, the same as NaN.
• timedelta, pd.Timedelta and timedelta64 save as ISO-8601 durations, and pd.Timedelta parses them back.
• A NaN dictionary key raises TypeError, like the other rejections.
• The SaveJson docs now say that a NaN inside a .NET object is written as Newtonsoft's "NaN" string.
• Each case has a test, and the Python regression algorithm now stores a DatetimeIndex.values.

What my probe showed: every case below either saved the right value or raised a TypeError that names the problem. None of them saved wrong data and reported success.

• np.datetime64('2300-01-01') saved as "2300-01-01T00:00:00".
• An object-dtype array of Timestamps saved as ISO strings.
• datetime64 and Timestamp dictionary keys saved as ISO strings.
• A tz-aware Timestamp kept its offset.
• A tz-aware DatetimeIndex.values saved as the UTC instant. That's what numpy's .values holds, so it's expected.
• A pd.NaT key raised a TypeError.

Non-blocking notes:

• A 0-d datetime64 array, np.array(np.datetime64(...)), raises TypeError: iteration over a 0-d array. It fails loudly and it's rare, so you can leave it.
• A negative timedelta saves in pandas' form P-1DT23H0M0S. pd.Timedelta reads that back correctly, but other ISO-8601 parsers may not. One line in the user docs would cover it.

Other repositories: the Python stubs are generated. The documentation still needs the new ObjectStore members and error messages, as the description says.

Risk label: risk:high, unchanged. This PR adds members to Lean's user-facing algorithm API (SaveJson/ReadJson for Python objects, SaveDataframe/ReadDataframe, SaveText/ReadText, IsSupportedKey/ValidateKey/SanitizeKey) and changes the messages of existing errors. Additions to the algorithm API are high risk however well they're tested, so a person should make the merge decision.