Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(2.2 and 2.3) CSVMixin/PIMixin allows set timeseries unequal to import times #1122

Open
SGeeversAtVortech opened this issue Sep 7, 2019 · 0 comments

Comments

@SGeeversAtVortech
Copy link
Contributor

In GitLab by @vreeken on Sep 7, 2019, 13:32

Calling set_timeseries with a Timeseries object

CSVMixin should behave like PIMixin, but does not.

The relevant part in CSVMixin's set_timeseries:

        if isinstance(timeseries, Timeseries):
            # TODO: add better check on timeseries.times?
            if check_consistency:
                if not np.array_equal(self.times(), timeseries.times):
                    raise Exception(
                        'CSV: Trying to set/append timeseries {} with different times '
                        '(in seconds) than the imported timeseries. Please make sure the '
                        # ...

It should not check against self.times(), but against self.__timeseries_times_sec. What happens now is that doing a get_timeseries will return an object with len(ts.times) != len(ts.values):

    def get_timeseries(self, variable, ensemble_member=0):
        return Timeseries(self.__timeseries_times_sec, self.__timeseries[ensemble_member][variable])

In PIMixin, there's the following check:

                    if not set(self.__timeseries_import_times).issuperset(timeseries.times):

All missing values are filled with NaNs (before and after).

Calling set_timeseries with an np.ndarray object:

Now for the case where we call set_timeseries with an array.

PIMixin has:

            if check_consistency:
                try:
                    assert(len(self.times()) == len(timeseries))
                except AssertionError:

This is not sufficient. We should also check that self.__timeseries_import_times is a superset of self.times().

Same goes for CSVMixin:

        else:
            timeseries = Timeseries(self.times(), timeseries)
            assert(len(timeseries.times) == len(timeseries.values))

Again should check that it is a superset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant