-
Notifications
You must be signed in to change notification settings - Fork 11
Fix incorrect session grouping #904
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
Open
mohamedelabbas1996
wants to merge
33
commits into
main
Choose a base branch
from
fix/group-images-into-sessions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
ece53c1
Modify group_images_into_events to read new images and merge into exi…
mohamedelabbas1996 76a4cb4
Refactor group_images_into_events to simplify logic and reduce code d…
mohamedelabbas1996 9c9a216
Add tests for image grouping
mohamedelabbas1996 5249e77
Remove unused group_by field from Event model
mohamedelabbas1996 1b2d2a6
Removed _create_captures_from_url
mohamedelabbas1996 660e883
Merge branch 'main' into fix/group-images-into-sessions
mohamedelabbas1996 0bbcfdd
Added missing import
mohamedelabbas1996 4fb045a
feat: action to remove images and occurrences from events
mihow 7a4949c
feat: clear images and occurrences from events from event model
mihow a9213ee
fix: update count in logs
mihow 7cb4c6e
Refactor group_images_into_events: removed unnecessary checks and que…
mohamedelabbas1996 706c560
Merge branch 'fix/group-images-into-sessions' of https://github.com/R…
mohamedelabbas1996 9bd037f
fix: allow the group_by removal migration to be reversed
mihow 05ce90c
Merge branch 'fix/group-images-into-sessions' of github.com:RolnickLa…
mihow e0fcadc
test: add tests for full regrouping with updated time gap and new ove…
mohamedelabbas1996 aeee536
Merge branch 'fix/group-images-into-sessions' of https://github.com/R…
mohamedelabbas1996 5d8df37
Add admin action to fix sessions by regrouping images per deployment
mohamedelabbas1996 67ff8b1
feat: clean up when events are regrouped for a deployment
mihow ebcf866
Handle merging with following overlapping events
mohamedelabbas1996 bb14620
Merge branch 'fix/group-images-into-sessions' of https://github.com/R…
mohamedelabbas1996 5465b4d
Tried fixing broken tests
mohamedelabbas1996 20ed64c
Updated fix_sessions admin action to use dissociate_related_objects
mohamedelabbas1996 21e39eb
Updated fix_sessions admin action group_images_into_events to use_exi…
mohamedelabbas1996 a800802
Omit redundant use_existing=True from group_images_into_events call
mohamedelabbas1996 931a686
Set use_existing=False to ensure all images are regrouped, not just o…
mohamedelabbas1996 541f26b
Merge branch 'main' of github.com:RolnickLab/antenna into fix/group-i…
mihow 9ecf60c
Merge branch 'main' of github.com:RolnickLab/antenna into fix/group-i…
mihow f1ab1b4
chore: rebase migration
mihow 3583da5
fix: ignore existing events when testing grouping
mihow 21afc3c
chore: add type hint
mihow 1b46b35
Add assertions to ensure event.end is not None during grouping
mohamedelabbas1996 9808407
feat: management command to audit and update event groups
mihow 8ec6671
fix: delete empty events AFTER occurrences are updated
mihow File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |||||
|
|
||||||
| import ami.utils | ||||||
| from ami import tasks | ||||||
| from ami.main.models import group_images_into_events, update_calculated_fields_for_events | ||||||
| from ami.ml.models.project_pipeline_config import ProjectPipelineConfig | ||||||
| from ami.ml.tasks import remove_duplicate_classifications | ||||||
|
|
||||||
|
|
@@ -20,6 +21,7 @@ | |||||
| Detection, | ||||||
| Device, | ||||||
| Event, | ||||||
| EventQuerySet, | ||||||
| Occurrence, | ||||||
| Project, | ||||||
| S3StorageSource, | ||||||
|
|
@@ -43,7 +45,7 @@ class AdminBase(admin.ModelAdmin): | |||||
| readonly_fields = ("created_at", "updated_at") | ||||||
|
|
||||||
| @admin.action(description="Save selected instances in the background") | ||||||
| def save_async(self, request: HttpRequest, queryset: QuerySet[SourceImage]) -> None: | ||||||
| def save_async(self, request: HttpRequest, queryset: QuerySet[Any]) -> None: | ||||||
| app_label = self.model._meta.app_label | ||||||
| model_name = self.model._meta.model_name | ||||||
| assert app_label and model_name, "Model must have app_label and model_name" | ||||||
|
|
@@ -239,13 +241,35 @@ def duration_display(self, obj) -> str: | |||||
| # Save all events in queryset | ||||||
| @admin.action(description="Updated pre-calculated fields") | ||||||
| def update_calculated_fields(self, request: HttpRequest, queryset: QuerySet[Event]) -> None: | ||||||
| from ami.main.models import update_calculated_fields_for_events | ||||||
|
|
||||||
| update_calculated_fields_for_events(qs=queryset) | ||||||
| self.message_user(request, f"Updated {queryset.count()} events.") | ||||||
|
|
||||||
| @admin.action() | ||||||
| def dissociate_related_objects(self, request: HttpRequest, queryset: EventQuerySet) -> None: | ||||||
| """ | ||||||
| Remove source images and occurrences from events. | ||||||
|
|
||||||
| This is useful when you want to recalculate events from source images. | ||||||
| """ | ||||||
| queryset.dissociate_related_objects() | ||||||
| self.message_user(request, f"Dissociated {queryset.count()} events from captures and occurrences.") | ||||||
|
|
||||||
| @admin.action(description="Fix sessions by regrouping images") | ||||||
| def fix_sessions(self, request: HttpRequest, queryset: EventQuerySet) -> None: | ||||||
| queryset.dissociate_related_objects() | ||||||
| # Get unique deployments from the selected events | ||||||
| deployments = Deployment.objects.filter(events__in=queryset).distinct() | ||||||
|
|
||||||
| # Regroup images for each deployment | ||||||
| for deployment in deployments: | ||||||
| # use_existing=False to consider regrouping all images, | ||||||
| # not just images without an assigned event (newly add images) | ||||||
| group_images_into_events(deployment, use_existing=False) | ||||||
|
|
||||||
| self.message_user(request, f"Fixed sessions: regrouped images in {len(deployments)} deployment(s).") | ||||||
|
|
||||||
| list_filter = ("deployment", "project", "start") | ||||||
| actions = [update_calculated_fields] | ||||||
| actions = [fix_sessions, dissociate_related_objects, update_calculated_fields] | ||||||
|
|
||||||
|
|
||||||
| @admin.register(SourceImage) | ||||||
|
|
@@ -274,10 +298,7 @@ class SourceImageAdmin(AdminBase): | |||||
| "collections", | ||||||
| ) | ||||||
|
|
||||||
| search_fields = ( | ||||||
| "id", | ||||||
| "path", | ||||||
| ) | ||||||
| search_fields = ("id", "path", "event__start__date") | ||||||
|
||||||
| search_fields = ("id", "path", "event__start__date") | |
| search_fields = ("id", "path") |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 like having a dedicated
fix_sessionsaction. However you can use the function above (queryset.dissociate_related_objects()) to remove images and occurrences from the Event. That's what I designed it for. Something like:I think
use_existing=Trueworks in this case