Can we make ModeOfTransport a string?

[Jammy2211]

class ModeOfTransport:
    __all = dict()

    def __new__(cls, description):
        if description not in ModeOfTransport.__all:
            ModeOfTransport.__all[
                description
            ] = super().__new__(cls)
        return ModeOfTransport.__all[
            description
        ]

    def __init__(
            self,
            description
    ):
        self.description = description

    def index(self, headers: List[str]) -> int:
        """
        Determine the column index of this mode of transport.

        The first header that contains this mode of transport's description
        is counted.

        Parameters
        ----------
        headers
            A list of headers from a CSV file.

        Returns
        -------
        The column index corresponding to this mode of transport.

        Raises
        ------
        An assertion error if no such header is found.
        """
        for i, header in enumerate(headers):
            if self.description in header:
                return i
        raise AssertionError(
            f"{self} not found in headers {headers}"
        )

    def __eq__(self, other):
        if isinstance(other, str):
            return self.description == other
        if isinstance(other, ModeOfTransport):
            return self.description == other.description
        return super().__eq__(other)

    def __hash__(self):
        return hash(self.description)

    def __str__(self):
        return self.description

    def __repr__(self):
        return f"<{self.__class__.__name__} {self}>"

    @classmethod
    def load_from_file(
            cls,
            config_filename=default_config_filename
    ) -> List["ModeOfTransport"]:
        """
        Load all of the modes of transport from commute.yaml.

        Modes of transport are globally unique. That is, even if the function
        is called twice identical mode of transport objects are returned.

        Parameters
        ----------
        config_filename
            The path to the mode of transport yaml configuration

        Returns
        -------
        A list of modes of transport
        """
        with open(config_filename) as f:
            configs = yaml.load(f, Loader=yaml.FullLoader)
        return [
            ModeOfTransport(
                config["description"]
            )
            for config in configs
        ]

Is there any reason that this needs to be a class as opposed to a string? Looking within the commute module it seems a bit overkill to make this a class, assuming that:

• We can load it from configs as needs be in functions that handle the configs.
• The index function can likely be moved to whatever class actually uses it (e.g. this seems to just be when loading configs, so can probably be performed during config generation).

Once I'm more familiar with the code-base I'll have a go at sorting this.

[rhayes777]

Yeah it's probably over-engineered.

I guess there are two reasons for having it as a class:

1. You can check its type
2. You might want to add richer functionality.

For example, you might also associate a transmission constant with each form of transport that could be comprised by this class.

However, in the short term there isn't much motivation for having it as a class instead of a string.

[arnauqb]

I think it is good to leave it as a class since, as Richard says, the aim is to have interaction intensities associated to different means of transport.

[florpi]

Don't we only care about the distinction between public and private transport? Although perhaps we want to make something more precise (like buses are smaller than subway trains). Although in the end you should more or less interact with the same number of people in both, there are some subtle distinctions like you don't interact with everyone in the same subway train, but the subway is closer than a bus and could potentially help the spread of the disease. We need to think about it

[FrankKrauss]

Let us for the time being only go public vs. private.

We can later become more specific - so, structurally I would make it a class, with currently only two instances.

On 24/04/2020 17:14, florpi wrote:

Don't we only care about the distinction between public and private transport? Although perhaps we want to make something more precise (like buses are smaller than subway trains). Although in the end you should more or less interact with the same number of people in both, there are some subtle distinctions like you don't interact with everyone in the same subway train, but the subway is closer than a bus and could potentially help the spread of the disease. We need to think about it

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JosephPB/covidmodelling/issues/32#issuecomment-619107121, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBDWIHYL5N6RY2QQNWEL6TROG3G5ANCNFSM4MQAKTAQ.

--

Professor Frank Krauss

Royal Society Wolfson Fellow

Institute for Particle Physics Phenomenology Director, Institute for Data Science Durham University

Durham DH1 3LE United Kingdom

Tel: +44 191 3343751 https://www.ippp.dur.ac.uk/profile/krauss

[rhayes777]

Better yet we can add a public/private attribute in the configuration. We would still know the specific form of transport.

On 24 Apr 2020, at 17:26, FrankKrauss notifications@github.com wrote:

Let us for the time being only go public vs. private.

We can later become more specific - so, structurally I would make it a class, with currently only two instances.

On 24/04/2020 17:14, florpi wrote:

Don't we only care about the distinction between public and private transport? Although perhaps we want to make something more precise (like buses are smaller than subway trains). Although in the end you should more or less interact with the same number of people in both, there are some subtle distinctions like you don't interact with everyone in the same subway train, but the subway is closer than a bus and could potentially help the spread of the disease. We need to think about it

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/JosephPB/covidmodelling/issues/32#issuecomment-619107121, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBDWIHYL5N6RY2QQNWEL6TROG3G5ANCNFSM4MQAKTAQ.

--

Professor Frank Krauss

Royal Society Wolfson Fellow

Institute for Particle Physics Phenomenology Director, Institute for Data Science Durham University

Durham DH1 3LE United Kingdom

Tel: +44 191 3343751 https://www.ippp.dur.ac.uk/profile/krauss

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JosephPB/covidmodelling/issues/32#issuecomment-619113848, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQRGMQI3SBBWETB4I4CA7TROG4TZANCNFSM4MQAKTAQ.

[rhayes777]

https://github.com/JosephPB/covidmodelling/pull/40 adds is_public to each mode of transport.