[tech] Easier hello world

[antoine-de]

The current API to load a model (from a GTFS or NTFS) is honestly quite straightforward and easy. I think we can still improve it a bit and I think a very neat library example can ease adoption.

For the moment to load a Model from a GTFS (and I think it can be applied to the other format) you need to do:

let model = gtfs::read_from_path(input_dir, gtfs::Configuration::default())?;

I think here the gtfs::Configuration::default() could be hidden.
Moreover in most uses of gtfs (like all transit_model's executables) the model is initialized like:

    let model = if opt.input.is_file() {
        transit_model::gtfs::read_from_zip(opt.input, configuration)?
    } else if opt.input.is_dir() {
        transit_model::gtfs::read_from_path(opt.input, configuration)?
    } else {
        bail!("Invalid input data: must be an existing directory or a ZIP archive");
    };

since it's a common use, it could be done for the users.

To ease the canonical way to create a model I think we can:

• add a magic function that can read a zip or path (and urls in the futur ? 😻 )
• add a builder pattern to hide the configuration

Pseudo code for this

First the usage:

The easiest

let model = gtfs::read("./pouet")?;
// or
let model = gtfs::read("./pouet.zip")?;
// or later
let model = gtfs::read("http://pouet.zip")?; :heart_eyes_cat: 

or directly

let model = gtfs::from_zip("./pouet.zip")?;

and with a custom config

let model = gtfs::Reader::default()
  .prefix("IDFM")
  .on_demand_transport(true)
  .read("./pouet")?;

And the gtfs module

mod gtfs {
  pub fn from_zip(p: impl AsRef<Path>) -> Result<Model> {
     gtfs::Reader::default().from_zip(p)
  }
  pub fn from_path(path: impl AsRef<Path>) -> Result<Model> {…}
  pub fn read(path: impl AsRef<Path>) -> Result<Model> {…} // read from a zip or a path
  pub fn from_reader(path: impl AsRef<Path>) -> Result<Model> {…} // see https://github.com/CanalTP/transit_model/issues/737
  
  pub struct Reader {
    configuration: transit_model::gtfs::Configuration
  }
  impl Reader {  
    // The 'builder' functions that 'eats' the `Reader`
    pub fn from(self, path: impl AsRef<Path>) -> Result<Model> {…} // not convinced by the name, would read from a zip or a path
    pub fn from_zip(self, path: impl AsRef<Path>) -> Result<Model> {…}
    pub fn from_path(self, path: impl AsRef<Path>) -> Result<Model> {…}
    pub fn from_reader(self, path: impl AsRef<Path>) -> Result<Model> {…} // see https://github.com/CanalTP/transit_model/issues/737
 
    // all needed functions to configure the transit_model::gtfs::Configuration
    pub fn prefix(self, p: &str) -> Self {…}
    pub fn contributor(self, c: &str) -> Self {…}
    // …
  }
}

What do you think about this? Do you think it is worth it? Any better ideas on the naming?

[woshilapin]

I think that would be a very nice API ❤️. Note that this is only about the GTFS API, maybe something similar for NTFS and the rest of the public API.

[pbougue]

It's a nice API, yes.
Not sure that from_zip and from_path would be used a lot (except for forcing one) as the perf price to have both is low.

At first sight, I would put it in 'polish' category, so my question for working on it is: what is the expected gain on that?
A lot of people would use the lib to do some opensource PT-stuffs that we would consider reusable?
(I compare it with the yak-shaving part of changing all the error-handling that seems more valuable, but I'm absolutely not confident in my judgement/ideas on that).

[antoine-de]

yes I agree the gain would be quite minimal, it can maybe bring some reusers and lower the step to use transit_model (even for CanalTP), but the time to implement this is also very limited so I guess so trade off is not bad.

For from_zip and from_path you're right they are only meant to be used if the default magic to call the right one is not ok for some specific use case.

[antoine-de]

After discussion with @woshilapin:

• parse_zip_read(read: impl Read, source_name: &str)
• parse_zip_path(path: impl AsRef<Path>)
• parse_dir_path(path: impl AsRef<Path>)
• parse(path: impl AsRef<Path>)

[datanel]

no parsing from reader?

[woshilapin]

The first one named parse_zip_read() takes a std::io::Read as input. And that is actually the reason why we started using the verb parse instead of read, to avoid something like read_zip_read(read: impl Read)

[datanel]

The first one named parse_zip_read() takes a std::io::Read as input. And that is actually the reason why we started using the verb parse instead of read, to avoid something like read_zip_read(read: impl Read)

what is source_name ?

[antoine-de]

it's uggly but it's an artifact since the FileHandler needs a name for better errors.