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

Make node marker and label sizes configurable via preferences #2057

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Make node marker and label sizes configurable via preferences #2057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
820e145
Make node marker and label sizes configurable via preferences
talmo Dec 16, 2024
df7f0cb
Fix test
talmo Dec 16, 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
5 changes: 3 additions & 2 deletions sleap/gui/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,17 +656,18 @@ def prev_vid():
key="edge style",
)

# XXX
add_submenu_choices(
menu=viewMenu,
title="Node Marker Size",
options=(1, 2, 4, 6, 8, 12),
options=prefs["node marker sizes"],
key="marker size",
)

add_submenu_choices(
menu=viewMenu,
title="Node Label Size",
options=(6, 12, 18, 24, 36),
options=prefs["node label sizes"],
key="node label size",
)

Expand Down
16 changes: 9 additions & 7 deletions sleap/prefs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class Preferences(object):
"node label size": 12,
"show non-visible nodes": True,
"share usage data": True,
"node marker sizes": (1, 2, 3, 4, 6, 8, 12),
"node label sizes": (6, 9, 12, 18, 24, 36),
}
_filename = "preferences.yaml"

Expand All @@ -43,14 +45,14 @@ def load_(self):
"""Load preferences from file (regardless of whether loaded already)."""
try:
self._prefs = util.get_config_yaml(self._filename)
if not hasattr(self._prefs, "get"):
self._prefs = self._defaults
else:
self._prefs["trail length"] = self._prefs.get(
"trail length", self._defaults["trail length"]
)
except FileNotFoundError:
self._prefs = self._defaults
pass

self._prefs = self._prefs or {}

for k, v in self._defaults.items():
if k not in self._prefs:
self._prefs[k] = v

def save(self):
"""Save preferences to file."""
Expand Down
7 changes: 3 additions & 4 deletions tests/gui/test_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -863,14 +863,13 @@ def test_DeleteFrameLimitPredictions(
context.state["video"] = centered_pair_vid

# Set-up params for the command
params = {"frame_idx_threshold": 900}
params = {"min_frame_idx": 900, "max_frame_idx": 1000}

expected_instances = 423
predicted_instances = DeleteFrameLimitPredictions.get_frame_instance_list(
instances_to_delete = DeleteFrameLimitPredictions.get_frame_instance_list(
context, params
)
Comment on lines +868 to 870
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the code analysis, I can now provide the final response:

Additional test cases needed for frame range deletion

The current implementation in DeleteFrameLimitPredictions lacks test coverage for important edge cases. While the UI (FrameRangeDialog) enforces valid ranges, the get_frame_instance_list method itself doesn't validate the input parameters. Specific test cases needed:

  • Equal bounds case: min_frame_idx equals max_frame_idx
  • Empty result case: Frame range with no instances
  • Invalid range case: min_frame_idx > max_frame_idx
  • Boundary cases: Frame indices at video start (1) and end

The implementation in sleap/gui/commands.py shows the method accepts the parameters without validation, making these test cases important for robustness.

🔗 Analysis chain

Verify test coverage for edge cases in frame range filtering.

While the basic functionality is tested, consider adding test cases for:

  • Edge cases where min_frame_idx equals max_frame_idx
  • Invalid ranges where min_frame_idx > max_frame_idx
  • Empty ranges where no instances exist

Let me gather more information about the test implementation to better understand the current coverage.

Let me check the implementation of DeleteFrameLimitPredictions class to understand how it handles edge cases.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for existing test coverage of edge cases

# Search for other test cases of DeleteFrameLimitPredictions
rg -A 5 "test_DeleteFrameLimitPredictions.*\(" 

# Look for any existing edge case tests
ast-grep --pattern 'assert $_($_, params={"min_frame_idx": $min, "max_frame_idx": $max})'

Length of output: 511

Script:

#!/bin/bash
# Get the full implementation of the test function and related code
rg -B 5 -A 20 "test_DeleteFrameLimitPredictions" tests/gui/test_commands.py

# Look for the implementation of get_frame_instance_list
ast-grep --pattern 'def get_frame_instance_list($$$)'

Length of output: 968

Script:

#!/bin/bash
# Find the DeleteFrameLimitPredictions class implementation
ast-grep --pattern 'class DeleteFrameLimitPredictions {
  $$$
}'

# Also search for any validation of frame indices
rg -B 2 -A 5 "min_frame_idx|max_frame_idx" --type py

Length of output: 4592


assert len(predicted_instances) == expected_instances
assert len(instances_to_delete) == 2070


@pytest.mark.parametrize("export_extension", [".json.zip", ".slp"])
Expand Down
Loading