Change adapters so that they can handle ascat timeseries and timezones #164
Conversation
|
Dunno what you mean by "try"... I've merged your branch into mine and the merge has no conflicts. I want to run the tests but I currently can't get them to work (in general), lots of "TypeError: boxcar_filter() got an unexpected keyword argument 'fillna'" But how would I see that the try is successful? |
|
There shouldn't be any conflict/complain about the timezone during the temporal matching, but maybe the timezone is also an issue somewhere else |
|
See issue #150 |
|
Cool! Can we then add that test back into the pytesmo tests? |
|
done 3cd2fb0 |
|
Great, thanks! Once your changes go to the master branch, |
| class MaskingAdapter(object): | ||
| class BasicAdapter(object): | ||
| """ | ||
| Base class for other adapters that works around the ascat reader not |
There was a problem hiding this comment.
Not just for ascat. Would work for any data in this format.
|
|
||
| def __get_dataframe(self, data): | ||
| if ((not isinstance(data, DataFrame)) and (hasattr(data, 'data')) and (isinstance(data.data, DataFrame))): | ||
| data = data.data |
There was a problem hiding this comment.
could add a key word argument for the name of the 'data', i.e. (hasattr(data, keyword_name_of_data)). Then if not named 'data', this could be overwritten. May be useful if a reader returns more than one data frame with data in it.
|
|
||
| def __drop_tz_info(self, data): | ||
| if data.index.tz is not None: | ||
| warnings.warn('Dropping timezone information for data') |
There was a problem hiding this comment.
Should we put in what the timezone was?
There was a problem hiding this comment.
i mean stating the timezone that has been dropped, i.e. UTC. May be a clearer message to the user. For example if they didn't know the timezones on the data going in, it would now tell them and may help them resolve errors more easily. Only a suggestion.
How do you mean? |
…mment, optional column name parameter, better warning message
|
OK, suggestions implemented. |
Remove old files in appveyor folder that are not used anymore Support x64 python 2 and 3 and x86 for py2
We install python via conda
Pulling in wpreimes's changes to appveyor config
|
@tracyscanlon or @sebhahn can you merge this please, everything ok from my side (even the windows builds work now), then #177 can be closed as well. |
|
I will look at the temporal matching merge request this week. Then we can hopefully merge it and remove the time zone dropping from this one. |
|
I've merged it now because:
|
Ascat returns data not in a dataframe but in an ascat timeseries object. This produces an exception if pytesmo tries to access the data's columns. This PR works around that.
Also, if data readers add timezone information to data, this PR normalizes to UTC and then drops tz info.