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.

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

Implementation of annotation ID option in objects getters #108

Merged
merged 6 commits into from
Aug 14, 2024

Conversation

Tom-TBT
Copy link
Contributor

@Tom-TBT Tom-TBT commented Jun 7, 2024

Description

Hello,
this implements an optional annotation parameter for the object getters. Providing an annotation ID, it retrieves the IDs of objects annotated with this annotation.

# Return IDs of all images annotated with Tag ID 876:
 >>> tag_ims = get_image_ids(conn, annotation=876)

This is my other attempt to get objects by annotations, instead of #88. In this case, I don't have a new function and I use HQL queries.

Feedback very much appreciated and thank you for your time

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.

@erickmartins
Copy link
Collaborator

(just FYI, I am will be on vacation for the next 3 weeks, so reviewing this might be a while!)

@erickmartins
Copy link
Collaborator

erickmartins commented Jul 25, 2024

ok, I think I understand this use case now and I'm on board with adding this. I'll make some time tomorrow to review code in a bit more detail, but I think this does allow for things that a simple filter_by_annotation() would not be able to achieve. (I also think we should replace filter_by_tag_value() and filter_by_kv() by a generic filter_by_annotation() that would allow generic filtering of ID lists by "which ones are linked to a given annotation ID", but that can wait)

@Tom-TBT
Copy link
Contributor Author

Tom-TBT commented Jul 26, 2024

This is great, I am glad that you are on board.
I will be away for two weeks for some holidays, so I'll look at your review when I'm back

Cheers

Copy link
Collaborator

@erickmartins erickmartins left a comment

Choose a reason for hiding this comment

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

Nothing to add or change on my end - I'll wait until you're back from vacation to merge it just in case

@Tom-TBT
Copy link
Contributor Author

Tom-TBT commented Aug 14, 2024

Hey Erick, I'm back from my holidays. I don't have any comment to add. You can go ahead.

@erickmartins erickmartins merged commit 8734008 into TheJacksonLaboratory:main Aug 14, 2024
1 check passed
@Tom-TBT Tom-TBT deleted the get_from_ann branch August 14, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants