Move from TravelTimeMatrixComputer etc. classes to TravelTimeMatrix inheriting from pandas.DataFrame/geopandas.GeoDataFrame

[wklumpen]

We have the code as written in the documentation, so let's perhaps make a helper function that does as follows:

travel_details["transport_mode"] = travel_details.transport_mode.astype(str)
travel_details["travel time (min)"] = travel_details.travel_time.apply(
    lambda t: round(t.total_seconds() / 60.0, 2)
)
travel_details["wait time (min)"] = travel_details.wait_time.apply(
    lambda t: round(t.total_seconds() / 60.0, 2)
)

# keep all columns except travel time and wait time (which we renamed to
# reflect the unit of measurement)
travel_details = travel_details[
    [
        "from_id",
        "to_id",
        "option",
        "segment",
        "transport_mode",
        "departure_time",
        "distance",
        "travel time (min)",
        "wait time (min)",
        "route",
        "geometry",
    ]
]

Something like r5py.utils.details_to_text(df) or even passing an argument.

[christophfink]

Hm, not sure - it's simple code that people can vopy-and-paste almost verbatim, but it would require more diligence and probably maintenance if we add this to our code.

On a similar note: I thought whether it would be smart to replace the '...Computer' classes with just 'TravelTimeMatrix', 'DetailedItineraries', and make them inherit from pandas.DataFrame/geopandas.GeoDataFrame, i.e., emphasise the output rather than the tool. Then we could also add all kind of (transparent) __-functions for typecasting and arithmetic operations (including child-class wrappesr of to_file() and explore() that handle the column type conversion before calling it's parent

(That would be very pythonic, but a breaking change - which we could introduce now still, but which will be difficult later on. Of course we should add a thin wrapper named like the old 'Computer' class that raises a DeprecationWarning)

[christophfink]

Sorry for the typos, writing on my phone

[wklumpen]

I actually really like the idea of simplifying the names and for using inheritance; would it be a significant (potentially worth it) departure from r5r?

[christophfink]

I actually really like the idea of simplifying the names and for using inheritance; would it be a significant (potentially worth it) departure from r5r?

I wouldn't think so - R is function-oriented, while Python is an object-oriented language. This difference is reflected in the current differences, already, the change would not be significant, really

[wklumpen]

I'm all for it. Do we need a 3rd? @HTenkanen?

[wklumpen]

Since this is a breaking namespace change I suggest this should probably be a target for v0.2 since this technically falls under a MAJOR change according to semantic versioning, but we don't have a MAJOR increment yet until we're at 1.0

[christophfink]

Absolutely. We should do this soon, though, precisely because it is a breaking change (even though I’d propose wrappers that raise a DeprecationWarning but work as previously). Should we increase the version number of the ‘paper’ release then, to 0.2.0

[wklumpen]

Am in favour of this change

[HTenkanen]

I'm also supporting this change to shorter names 👍🏻

[wklumpen]

For consideration, an alternative way to extend without inheritance:

https://pandas.pydata.org/docs/development/extending.html

[christophfink]

For consideration, an alternative way to extend without inheritance:

https://pandas.pydata.org/docs/development/extending.html

This looks enticing! Have to take a closer look :)

[wklumpen]

Having played around with it now I'm not sure it fits our model - it just adds separate methods to the namespace under a sub-space

GeoPandas just extends the actual class which you can do with some additional namespace stuff.