-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Fix comparing the deltas with current data for some some datatypes, mostly dates and times #588
Conversation
…ostly dates and times `QgsFeature.attribute()` will return an instance of `QDate`, `QTime` or `QDateTime`, depending on the attribute datatype. We need to explicitly handle such situations. Note geopackages does not support `time` fields, so this case is untested, but should follow the same logic as date/datetime.
…and lists the conflicts in the job output
…atch "sourcePk" in the delta"
if feature.attribute(attr) != delta_feature_attrs[attr]: | ||
current_value = feature.attribute(attr) | ||
incoming_value = delta_feature_attrs[attr] | ||
|
||
# modify the incoming value to the desired type if needed | ||
if incoming_value is not None: | ||
if isinstance(current_value, QDateTime): | ||
incoming_value = QDateTime.fromString( | ||
incoming_value, Qt.ISODateWithMs | ||
) | ||
elif isinstance(current_value, QDate): | ||
incoming_value = QDate.fromString(incoming_value, Qt.ISODate) | ||
elif isinstance(current_value, QTime): | ||
incoming_value = QTime.fromString(incoming_value) | ||
|
||
if current_value != incoming_value: | ||
conflicts.append( | ||
f'The attribute "{attr}" that has a conflict:\n-{delta_feature_attrs[attr]}\n+{feature.attribute(attr)}' | ||
f'The attribute "{attr}" that has a conflict:\n-{current_value}\n+{incoming_value}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m-kuhn kind request for review. If I understand correctly, there are no more interesting return value types from QgsFeature.attribute(QString)
. Tried with binary and json, also checked QVariant
docs, and seems everything else will be a datatype supported from JSON (as incoming_value
is read from delta's JSON).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is correct
QgsFeature.attribute()
will return an instance ofQDate
,QTime
orQDateTime
,depending on the attribute datatype. We need to explicitly handle such situations.
Note geopackages does not support
time
fields, so this case is untested, but should follow the same logic as date/datetime.Fixes #572 .