Increase test coverage of solph.models module

[uvchik]

There are some special conditions in this module and we should remove them or test them.

Furthermore custom.py is removed from the test coverage check, because we announced that it is not necessary to write tests for the custom components but we should warn users about this.

[coveralls]

Coverage increased (+3.5%) to 91.763% when pulling 9884d07 on features/increase_test_coverage_of_model_module into 5bcb7d7 on releases/v0_3_0.

[uvchik]

First thing to discuss in this context:

At the moment a timeincrement of one is set if the time increment wasn't defined correctly. If I have a time increment of 15 minutes but made a mistake the time increment would be set to one in the background and all I get is a logging warning.

To leave the opportunity to create your own time increment I would add the possibility to pass it directly to the model, otherwise solph tries to get it from the timeindex and if this is not possible an error is raised. If you want we could add our own error message but an error should be raised in my opinion.

ACTUAL VERSION:

    def __init__(self, energysystem, **kwargs):
        super().__init__()
        self.name = kwargs.get('name', type(self).__name__)
        self.es = energysystem
        try:
            self.timeincrement = sequence(self.es.timeindex.freq.nanos / 3.6e12)
        except AttributeError:
            logging.warning(
                'Could not get timeincrement from pd.DateTimeIndex! ' +
                'To avoid this warning, make sure the `freq` attribute of ' +
                'your timeindex is not None. Setting timeincrement to 1...')
            self.timeincrement = sequence(1)

MY PROPOSAL:

    def __init__(self, energysystem, **kwargs):
        super().__init__()
        self.name = kwargs.get('name', type(self).__name__)
        self.es = energysystem
        self.timeincrement = kwargs.get(timeincrement, None)

        If self.timeincrement is None:
            self.timeincrement = sequence(self.es.timeindex.freq.nanos / 3.6e12)

The if block could also look like this to make it more clear for the user.

        If self.timeincrement is None:
            try:
                self.timeincrement = sequence(self.es.timeindex.freq.nanos / 3.6e12)
            except AttributeError:
                raise AttributeError(our_message)

I stumbled over this issue while I was thinking about how to test these lines.
@oemof/oemof-solph What do you think?

[uvchik]

As nobody commented the idea above I pushed a concrete proposal. If nobody objects I will merge.

[uvchik]

@oemof/oemof-solph See commit 2f6430 to review the changes on the time increment.

[simnh]

Will it still be possible to set a timeindex with length one?

[uvchik]

Since v0.2.1 it is possible and since PR #519 it is tested. If all tests pass it is possible

The cleanest way is the following:

datetimeindex = pd.date_range('1/1/2012', periods=1, freq='H')
energysystem = EnergySystem(timeindex=datetimeindex)
model = solph.Model(energysystem)

Until now the following code also worked but I do not like it because it is hidden magic.

energysystem = EnergySystem()
model = solph.Model(energysystem)

With commit fb316ca the default timeindex of an EnergySystem object is None and NOT pd.date_range(start=pd.to_datetime('today'), periods=1, freq='H') as it was before.

[uvchik]

@simnh Do have any objections? May you approve this?