-
Notifications
You must be signed in to change notification settings - Fork 6
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
Load db Interval column as ISO 8601 duration string rather than Python timedelta #141
Comments
Cool stuff! 😄 Following your steps, I came up with a way to avoid creating a new column. Tech spike: from sqlalchemy import types,func
from sqlalchemy.orm import column_property, declared_attr
from pandas.tseries.frequencies import to_offset
[...]
class SensorDBMixin(Sensor):
[...]
# rename event_resolution to _event_resolution
_event_resolution = Column("event_resolution", Interval(), nullable=False, default=timedelta(hours=0))
[...]
@declared_attr
def event_resolution_str(self):
""" Create a column property that casts event_resolution to String """
return column_property(func.cast(self.event_resolution, String))
@hybrid_property
def event_resolution(self):
"""Apply conversion from string to panda timedelta. TODO: use `postgres_interval_to_iso8601`"""
if "year" in self.event_resolution_str:
return to_offset("1Y")
t = event_resolution_str.split(":")
return timedelta(hours=int(t[0]), minutes=int(t[1]), seconds=int(t[2]))
@event_resolution.expression
def event_resolution(cls):
"""Map event_resolution expression to _event_resolution field. This is important to support things like:
session.execute(select(Sensor.id, Sensor.event_resolution))
"""
return cls._event_resolution
@event_resolution.setter
def event_resolution(self, value):
"""Map the setter of event_resolution to _event_resolution"""
self._event_resolution = value I tested the following: >>> from flexmeasures.data.models.time_series import Sensor
>>> from sqlalchemy import cast, String, select
>>> session = app.db.session
>>>
>>> s = Sensor.query.get(1)
>>> s.event_resolution
<YearEnd: month=12>
>>> s.event_resolution_str
'1 year'
>>> s.event_resolution = "P1Y"
>>> session.commit()
>>> s = Sensor.query.get(1)
>>> s.event_resolution
<YearEnd: month=12>
>>> print(select(Sensor.event_resolution))
SELECT sensor.event_resolution
FROM sensor
>>> session.execute(select(Sensor.event_resolution))
<sqlalchemy.engine.result.ChunkedIteratorResult object at 0x7ff1509bf190>
>>> session.execute(select(cast(Sensor._event_resolution, String))).all()
[('00:15:00',), ('00:15:00',), ('00:15:00',), ('00:15:00',), ('00:15:00',), ('1 year',), ('00:15:00',), ('00:15:00',), ('00:15:00',), ('00:15:00',), ('1 year',)]
>>> session.execute(select(cast(Sensor.event_resolution, String))).all()
[('00:15:00',), ('00:15:00',), ('00:15:00',), ('00:15:00',), ('00:15:00',), ('1 year',), ('00:15:00',), ('00:15:00',), ('00:15:00',), ('00:15:00',), ('1 year',)] |
This morning, @Flix6x and I had a meeting to discuss further that type to describe the event resolution. For the record:
If TODOs:
Feel free to add anything to this 😄 |
When initializing a
DBSensor
with an ISO 8601 duration string as itsevent_resolution
, this is correctly interpreted in Postgres. However, when loading the sensor from the database to a Python object, the event resolution becomes a Python timedelta, which cannot accurately represent nominal durations such as a calendar day, a month or a year. This is problematic for daily, monthly and yearly sensors, and also why timely-beliefs currently still works best for hourly or more fine-grained data.For example:
Becomes
1 mon
in Postgres, which then becomestimedelta(days=30)
in Python.At first, we'd just like to make it possible to let Python code access the original ISO 8601 representation of the event resolution, from what is stored in the database. Here are my ideas:
1) Postgres intervalStyle setting
Although Postgres's
intervalStyle
does a great job at formatting the stored Interval as an ISO string, unfortunately this setting is only scoped to a database connection and not meant to let SQLAlchemy consistently load the Interval as an ISO string.which yields "P1M". Given that the intervalStyle cannot be set globally, this functionality is not trivial to use for our purpose.
Update: it is actually possible to set the default for a given database, using:
Here's a possible migration we could consider:
However, we then run into this error:
2) String rather than Interval column
This does not allow the column to be used for db-level computation, unless it is cast to an Interval before doing computations. This idea still needs a tech spike.
3) Cast to string and convert upon loading
My current idea is to give the
Sensor
object an extra property, something likeevent_resolution_string
, get the string representation of theevent_resolution
from the database, and deal with the different possible intervalStyles upon loading.Tech spike:
And some ideas for test cases:
We should also test updating the sensor resolution, given this potential issue.
The text was updated successfully, but these errors were encountered: