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.

Sign up for GitHub

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 empty database missing 'modified' time causing table to not show #1182

Merged
merged 3 commits into from
Jan 9, 2024
Merged

Fix empty database missing 'modified' time causing table to not show #1182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa1e90d
fix empty database missing 'modified' time
marc-vdm Dec 27, 2023
3ae7e5c
Add tooltip for datetime as actual date instead of the humanized data.
marc-vdm Dec 27, 2023
cc0e162
Write proper date as tooltip
marc-vdm Dec 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions activity_browser/controllers/database.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def add_database(self):
if ok and name:
if name not in bw.databases:
bw.Database(name).register()
bw.Database(name).write({}) # write nothing to the database so we set a modified time
project_settings.add_db(name, False)
signals.databases_changed.emit()
signals.database_selected.emit(name)
Expand Down
23 changes: 17 additions & 6 deletions activity_browser/ui/tables/models/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ def data(self, index, role=Qt.DisplayRole):

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A switch case block also comes to mind. Those are awfully quick, meant for enums like these, and easy to read. Just spitballing

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the whole date thing could go in a child class for the DB table as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

# instantiate value only in case of DisplayRole or ToolTipRole
value = None
tt_date_flag = False # flag to indicate if value is datetime object and role is ToolTipRole
if role == Qt.DisplayRole or role == Qt.ToolTipRole or role == 'sorting':
value = self._dataframe.iat[index.row(), index.column()]
if isinstance(value, np.float64):
Expand All @@ -64,14 +65,23 @@ def data(self, index, role=Qt.DisplayRole):
value = value.item()
elif isinstance(value, tuple):
value = str(value)
elif isinstance(value, datetime.datetime) and role != 'sorting':
elif isinstance(value, datetime.datetime) and (Qt.DisplayRole or Qt.ToolTipRole):
tz = datetime.datetime.now(datetime.timezone.utc).astimezone()
time_shift = - tz.utcoffset().total_seconds()
value = arrow.get(value).shift(seconds=time_shift).humanize()
if role == Qt.ToolTipRole:
value = arrow.get(value).shift(seconds=time_shift).format('YYYY-MM-DD HH:mm:ss')
tt_date_flag = True
elif role == Qt.DisplayRole:
value = arrow.get(value).shift(seconds=time_shift).humanize()

# immediately return value in case of DisplayRole or sorting
if role == Qt.DisplayRole or role == 'sorting':
return value

# in case of ToolTipRole and date, always show the full date
if tt_date_flag and role == Qt.ToolTipRole:
return value
Comment on lines +82 to +83
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you just return the value where you've set the flag? That would clear the code up nicely

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case you wouldn't need the flag to begin with.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


# immediately return value in case of DisplayRole
if role == Qt.DisplayRole or role == 'sorting': return value

# in case of ToolTipRole, check whether content fits the cell
if role == Qt.ToolTipRole:
parent = self.parent()
Expand All @@ -83,7 +93,8 @@ def data(self, index, role=Qt.DisplayRole):
margin = 10

# only show tooltip if the text is wider then the cell minus the margin
if text_width > column_width - margin: return value
if text_width > column_width - margin:
return value

if role == Qt.ForegroundRole:
col_name = self._dataframe.columns[index.column()]
Expand Down
9 changes: 4 additions & 5 deletions activity_browser/ui/tables/models/inventory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,12 @@ def get_db_name(self, proxy: QModelIndex) -> str:
return self._dataframe.iat[idx.row(), 0]

def sync(self):
# code below is based on the assumption that bw uses utc timestamps
tz = datetime.datetime.now(datetime.timezone.utc).astimezone()
time_shift = - tz.utcoffset().total_seconds()

data = []
for name in natural_sort(bw.databases):
dt = datetime.datetime.strptime(bw.databases[name].get("modified", ""), '%Y-%m-%dT%H:%M:%S.%f')
# get the modified time, in case it doesn't exist, just write 'now' in the correct format
dt = bw.databases[name].get("modified", datetime.datetime.now().isoformat())
dt = datetime.datetime.strptime(dt, '%Y-%m-%dT%H:%M:%S.%f')

# final column includes interactive checkbox which shows read-only state of db
database_read_only = project_settings.db_is_readonly(name)
data.append({
Expand Down