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

Getters for High-Content-Sreening #101

Merged
merged 7 commits into from
Jun 5, 2024

Conversation

Tom-TBT
Copy link
Contributor

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

Description

The PR implements getters for Screen, Plate, Run, and Well, in the same style as the Project, Dataset, and Image getters.

I implemented tests to cover the new getter functions, for which I modified the generated screen.

There is in addition a new option for get_image_ids, to list images according to a Run ID. This could be a breaking change for anyone who specifies each parameter without their name. (I did that for the sake of consistency for the options order)

Breaking change case, the across_groups parameter was passed without its name:
get_image_ids(conn, None, None, 12, None, False)
This would now return a TypeError('Run ID must be integer') as the False value gets into the run parameter.

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

(this is just a message to acknowledge I've seen this and #102 - will get to these as soon as I have some time!

@erickmartins erickmartins self-assigned this May 30, 2024
@erickmartins
Copy link
Collaborator

picking nits: I think I'd rather have everything be plate_acquisition instead of run - the OME data model object is PlateAcquisition and I'd rather stick to the data model whenever possible!

@Tom-TBT
Copy link
Contributor Author

Tom-TBT commented Jun 4, 2024

I'll write here in case you haven't got a notification, I pushed changes to replace run with plate_acquisition :D

@erickmartins erickmartins merged commit c9d5757 into TheJacksonLaboratory:main Jun 5, 2024
1 check passed
@erickmartins
Copy link
Collaborator

Merged - thanks for the patience (and sorry for the communication issues!)

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