feat(closes OPEN-5988): openlayer-ts should accept csv datasets#178
feat(closes OPEN-5988): openlayer-ts should accept csv datasets#178shah-siddd wants to merge 1 commit intomainfrom
Conversation
shah-siddd
force-pushed
the
siddhant/open-5988-openlayer-ts-should-accept-csv-datasets
branch
2 times, most recently
from
fbd3d1a to
40c836a
Compare
shah-siddd
force-pushed
the
siddhant/open-5988-openlayer-ts-should-accept-csv-datasets
branch
from
40c836a to
9b6c6c2
Compare
gustavocidornelas
left a comment
gustavocidornelas
left a comment
There was a problem hiding this comment.
Added a few comments to version.ts.
Also, can you discard all changes outside of src/lib? Everything outside of it is auto-generated by Stainless, so this will cause issues. Version updates and changelog are fully handled by them based on the commit messages.
I recently wrote more details about SDK maintenance. I'm sure you know most of the things but sending here just in case.
Also, in the PR description, you mention unit tests but I don't see any. Make sure the description is accurate
There was a problem hiding this comment.
can you discard these changes?
There was a problem hiding this comment.
can you discard these changes?
There was a problem hiding this comment.
can you discard these changes?
There was a problem hiding this comment.
can you discard these changes?
There was a problem hiding this comment.
can you discard these changes?
| export interface RunReturn { | ||
| otherFields: { [key: string]: any }; | ||
| output: any; | ||
| [key: string]: unknown; |
There was a problem hiding this comment.
why do you need this?
| return { data: parsed, format }; | ||
| } | ||
|
|
||
| export function writeDataset( |
There was a problem hiding this comment.
this function seems to not be used anywhere
There was a problem hiding this comment.
i understand that this would be used to potentially save csv and json. however, i'm not sure we need it. even if the user specifies a csv dataset, we could keep saving a dataset.json with the model outputs, right?
| const datasetFullPath = path.resolve(datasetPath); | ||
| const rawData = fs.readFileSync(datasetFullPath, 'utf8'); | ||
| const dataset = JSON.parse(rawData); | ||
| const { data: dataset, format } = loadDataset(datasetPath); |
There was a problem hiding this comment.
format seems to not be used anywhere
2 participants
Pull Request
Summary
Adds CSV format support to the CLI dataset loading and writing functionality. The CLI now automatically detects and handles both JSON and CSV datasets based on file extension, maintaining format consistency between input and output files.
Changes
parseCsv,parseCsvLine)serializeCsv,escapeCsvValue)detectDatasetFormat)loadDatasetto support both JSON and CSV formatswriteDatasetto preserve input format in output filesCLIHandlerto use the new format-aware dataset functionsContext
Previously, the CLI only supported JSON datasets. This change enables users to work with CSV datasets, which are commonly used in data science workflows. The implementation handles edge cases like quoted values, commas within fields, and escaped quotes according to RFC 4180 standards.
Testing
Test Coverage:
Monitoring
Notes
"") correctly