why do we need this?

Not sure what you mean by "this", if its the {n} it was to try and get clap to generate newlines, I've removed from the text anyway

Yeah, I meant the {n}

This is a very long comment. Did you write it or copy it from arrow? If the former, please revise heavily and shorten.

I wrote it. I've had another go, chopped out a lot of the text.

Instead of a path, I wonder whether it's better to expect a string instead of a file. What do you think?

Providing the schema as a string would be feasible, the down side is it could be unwieldy.

The process I employed to get to the defined schema I've been playing with was:

• Do a dry run with schema infer, cat output to local file
• Manually edit local schema file
• Run with schema read from local file

To receive the defined schema as a string that flow becomes:

• Do a dry run with schema infer, cat output to local file
• Manually edit local schema file
• Read file into env var
• Run with schema read from env var

I can't see anyone being keen to type the whole schema by hand at the command line... !
The defined schema for the file I've been playing with (only 5 columns) is:

{
  "fields": [
    {
      "name": "plant_id",
      "nullable": false,
      "type": {
        "name": "int",
        "bitWidth": 32,
        "isSigned": false
      },
      "children": []
    },
    {
      "name": "analysis_year",
      "nullable": false,
      "type": {
        "name": "int",
        "bitWidth": 32,
        "isSigned": false
      },
      "children": []
    },
    {
      "name": "risk_attribute_value",
      "nullable": true,
      "type": {
        "name": "floatingpoint",
        "precision": "SINGLE"
      },
      "children": []
    },
    {
      "name": "physical_risk_scenario_id",
      "nullable": false,
      "type": {
        "name": "int",
        "bitWidth": 8,
        "isSigned": false
      },
      "children": []
    },
    {
      "name": "physical_risk_type_id",
      "nullable": false,
      "type": {
        "name": "int",
        "bitWidth": 8,
        "isSigned": false
      },
      "children": []
    }
  ],
  "metadata": {}
}

I don't see any changes to the code where we used to infer the schema. Instead of copying the logic, please refactor the code so we reuse the logic.

The logic has changed.

In the first match statement of fn main gets a schema by either:

• reading it in from the defined schema file, which is the path taken if a schema file is provided on the command line
• inferring it from the source csv file, using "arrow::csv::reader::infer_file_schema"

NOTE: the original code uses "builder.infer_schema(opts.max_read_records)", whereas the new code is using an associated function on the reader struct.

At this point we don't yet have a configured reader (or even a reader builder).

We do have a schema which we can give to the reader builder in a few lines time, which leads into your comment further down about re-using the existing logic...

can we reuse the existing logic?

Given that the logic around getting a schema has changed such that we've already got a schema without needing a Reader or ReaderBuilder, this seems to be the cleanest (and closest to the original logic) approach.

If we've got a defined schema file, we've read it and built the schema without touching the source csv, the input File hasn't been touched and is ready to read.

If we don't have a defined schema file and the max records is a value that lets us infer a schema, we've inferred a schema using the infer method on the CsvReader struct (rather than using the ReaderBuilder), the input File has been read and wound back to start (exactly as in the original code).

If we don't have a defined schema file and the max records is zero, we have a schema that's all strings, the input File is ready to read (exactly as in the original code).

If I've missed something, please try and point it out to me again :)

Please fix grammar and make it easier to understand.

Revised, grammar checked in MS Word and a couple of online services, no issues reported.

I don't think this is good here. It's an obvious statement and not usually what I expect to read in cli docs.