Fix empty database missing 'modified' time causing table to not show #1182
Conversation
| @@ -54,6 +54,7 @@ def data(self, index, role=Qt.DisplayRole): | |||
|
|
|||
There was a problem hiding this comment.
This method has become very unclear over the last commits. Maybe we should chop it up into different functions/methods. e.g. a value function that returns the value.
There was a problem hiding this comment.
A switch case block also comes to mind. Those are awfully quick, meant for enums like these, and easy to read. Just spitballing
There was a problem hiding this comment.
Maybe the whole date thing could go in a child class for the DB table as well
There was a problem hiding this comment.
This method has become very unclear over the last commits. Maybe we should chop it up into different functions/methods. e.g. a value function that returns the value.
Agree, it's a bit messy atm
A switch case block also comes to mind. Those are awfully quick, meant for enums like these, and easy to read. Just spitballing
switch only exists from py=3.10, so we can't implement this until we drop support for 3.9. Though elif is essentially a switch, and I don't think the speed matters in this case.
Maybe the whole date thing could go in a child class for the DB table as well
Also agree
| if tt_date_flag and role == Qt.ToolTipRole: | ||
| return value |
There was a problem hiding this comment.
Can't you just return the value where you've set the flag? That would clear the code up nicely
There was a problem hiding this comment.
In that case you wouldn't need the flag to begin with.
There was a problem hiding this comment.
Can't you just return the value where you've set the flag? That would clear the code up nicely
That's what I had earlier, but thought this may be more consistent as there's no returns in other places
|
Ok, so I agree this is not great right now, however, I do think we should have something to resolve the issue in the next release, as this is very confusing for users if they suddenly can't see their databases due to something that seems entirely unrelated. What about we open an issue for a proper re-write but merge this? Or otherwise if you have time for a re-write before the next release that should also work. |
|
Oh or we could revert to this state and just make an issue for the time formatted tooltip, that's where the mess is right now. |
Good points. Let's rewrite it properly in another release, I'll merge this and create an issue. |
|
Note This issue has been implemented in the new release of Activity Browser 🚀 (version 2.9.4), you can get the new version by updating Activity Browser. Do you want to be notified of new releases of Activity Browser? Subscribe to our updates mailing list ✉. 🤖_beep boop! I'm a bot and this message was an automated action._ |
changes:
modifiedtime by writing nothing (empty dict) to the new database.modifiedis still missing (e.g. user created empty database in Brightway), we just write current time to themodifiedcolumn in AB.Checklist
bug,feature,ui,change,documentation,breaking,cias they show up in the changelog.