Added check to JsonReader::finish that infer_schema_len was not Some(0)
#10574
Conversation
(returns PolarsError::InvalidOperation if it is) Added test that this case is caught correctly Updated docs of JsonReader::infer_schema_len(...) to state that it applies to both JsonFormat::JsonLines and ::Json Clarified other error message in JsonReader::finish
|
I should mention that this fixes a panic that occurred when |
| /// Set the JSON reader to infer the schema of the file. Currently, this is only used when reading from | ||
| /// [`JsonFormat::JsonLines`], as [`JsonFormat::Json`] reads in the entire array anyway. | ||
| /// Set the number of records for the JSON reader to infer the schema of the file | ||
| /// from. |
There was a problem hiding this comment.
In your edit the "Currently, this is only used when reading from [JsonFormat::JsonLines], as [JsonFormat::Json] reads in the entire array anyway." got lost. Can you put that back?
There was a problem hiding this comment.
It changed — both formats now use infer_schema_len. JsonFormat::Json will read in the entire array, but can inspect a subset of records for schema inference.
There was a problem hiding this comment.
Hi, just following up on this. The removal of that part of the comment was intentional because the removed content was outdated. Now, both JsonFormat::JsonLines and JsonFormat::Json can use infer_schema_len to check only a subset of records when inferring the schema. (Not sure when it changed.)
|
Sorry, I accidentally closed this when I mucked with some branches. Should I open a separate PR? |
|
I can't reopen this PR because 'the branch was force-pushed or recreated', so you'll have to open a new PR I'm afraid. |
(returns
PolarsError::InvalidOperationif it is)Added test that this case is caught correctly
Updated docs of
JsonReader::infer_schema_len(...)to state that it applies to bothJsonFormat::JsonLinesand::JsonClarified other error message inJsonReader::finish