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

Separate the video name and its filepath columns in VideoTablesModel #2052

Merged
merged 18 commits into from
Dec 18, 2024
Merged

Separate the video name and its filepath columns in VideoTablesModel #2052

Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ac6da50
add option to show video names with filepath
7174Andy Dec 16, 2024
788e8bc
add doc
7174Andy Dec 16, 2024
a92fc2d
new feature added successfully
7174Andy Dec 17, 2024
2ea89bb
delete unnecessary code
7174Andy Dec 17, 2024
e03be7e
remove attributes from video object
7174Andy Dec 17, 2024
8cac64c
Update dataviews.py
7174Andy Dec 17, 2024
1fa5f02
remove all properties
7174Andy Dec 17, 2024
8e6b947
delete toggle option
7174Andy Dec 17, 2024
22dd69e
remove video show
7174Andy Dec 17, 2024
8db81a9
fix the order of the columns
7174Andy Dec 17, 2024
7529acc
remove options
7174Andy Dec 17, 2024
9d747cb
Update sleap/gui/dataviews.py
7174Andy Dec 17, 2024
d38c0c4
Update sleap/gui/dataviews.py
7174Andy Dec 17, 2024
946082d
use pathlib instead of substrings
7174Andy Dec 17, 2024
7044c21
Merge branch 'develop' into andrew/break-out-filenames-in-video-table
7174Andy Dec 17, 2024
d158633
Update dataviews.py
7174Andy Dec 18, 2024
69b6273
Use Path instead of pathlib.Path
roomrys Dec 18, 2024
5b1b7e9
Use item.filename instead of getattr
roomrys Dec 18, 2024
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
26 changes: 22 additions & 4 deletions sleap/gui/dataviews.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,28 @@ def getSelectedRowItem(self) -> Any:


class VideosTableModel(GenericTableModel):
properties = ("filename", "frames", "height", "width", "channels")

def item_to_data(self, obj, item):
return {key: getattr(item, key) for key in self.properties}
properties = (
"name",
"filepath",
"frames",
"height",
"width",
"channels",
)

def item_to_data(self, obj, item: "Video"):
data = {}
for property in self.properties:
if property == "name":
filename = getattr(item, "filename")
name = Path(filename).name
elif property == "filepath":
filename = getattr(item, "filename")
parent = Path(filename).parent
data[property] = "/".join(splitted)
else:
data[property] = getattr(item, property)
return data
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix critical issues in the item_to_data method implementation.

The current implementation has several issues:

  1. The extracted name and parent values are not used
  2. The undefined splitted variable is used
  3. Missing required imports
  4. No handling for potential None values

Apply this diff to fix the issues:

+from pathlib import Path
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from sleap.io.video import Video

 def item_to_data(self, obj, item: "Video"):
     data = {}
     for property in self.properties:
         if property == "name":
-            filename = getattr(item, "filename")
-            name = Path(filename).name
+            filename = item.filename
+            data[property] = Path(filename).name if filename else ""
         elif property == "filepath":
-            filename = getattr(item, "filename")
-            parent = Path(filename).parent
-            data[property] = "/".join(splitted)
+            filename = item.filename
+            data[property] = str(Path(filename).parent) if filename else ""
         else:
             data[property] = getattr(item, property)
     return data

Changes made:

  1. Added required imports
  2. Properly used Path for name and filepath extraction
  3. Added null checks for filename
  4. Removed unnecessary getattr calls
  5. Fixed the filepath construction using str(Path().parent)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def item_to_data(self, obj, item: "Video"):
data = {}
for property in self.properties:
if property == "name":
filename = getattr(item, "filename")
name = Path(filename).name
elif property == "filepath":
filename = getattr(item, "filename")
parent = Path(filename).parent
data[property] = "/".join(splitted)
else:
data[property] = getattr(item, property)
return data
from pathlib import Path
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from sleap.io.video import Video
def item_to_data(self, obj, item: "Video"):
data = {}
for property in self.properties:
if property == "name":
filename = item.filename
data[property] = Path(filename).name if filename else ""
elif property == "filepath":
filename = item.filename
data[property] = str(Path(filename).parent) if filename else ""
else:
data[property] = getattr(item, property)
return data
🧰 Tools
🪛 Ruff (0.8.2)

398-398: Undefined name Video

(F821)

402-402: Do not call getattr with a constant attribute value. It is not any safer than normal property access.

Replace getattr with attribute access

(B009)

403-403: Local variable name is assigned to but never used

Remove assignment to unused variable name

(F841)

403-403: Undefined name Path

(F821)

405-405: Do not call getattr with a constant attribute value. It is not any safer than normal property access.

Replace getattr with attribute access

(B009)

406-406: Local variable parent is assigned to but never used

Remove assignment to unused variable parent

(F841)

406-406: Undefined name Path

(F821)

407-407: Undefined name splitted

(F821)



class SkeletonNodesTableModel(GenericTableModel):
Expand Down
Loading