Skip to content

script_utils.getObjects() sorts by IDs#1202

Merged
joshmoore merged 1 commit intoome:dev_4_4from
will-moore:scripts_obj_sorting_10745
May 24, 2013
Merged

script_utils.getObjects() sorts by IDs#1202
joshmoore merged 1 commit intoome:dev_4_4from
will-moore:scripts_obj_sorting_10745

Conversation

@will-moore
Copy link
Copy Markdown
Member

See bug #10745. We now sort objects by IDs in script_utils.getObjects().

To test:

  • Select a bunch of images and open split view figure dialog in Web.
  • Re-order the images by dragging & dropping rows (handle to left of each row)
  • Grab a screen-shot (makes comparing easier)
  • Run script and check that the figure generated has the same order of rows as the preview.

@sbesson
Copy link
Copy Markdown
Member

sbesson commented May 23, 2013

In terms of logic, sorting objects returned by the server by input ids is sensible and should fix bugs in other scripts than the one ticketed.
Testing scenario described above was successfully tested against howe.

👍 Ready to merge.

NB: I tried to test this PR using the Combine Images script (with non-defaults parameters) and realized this script is not working anymore.

@will-moore
Copy link
Copy Markdown
Member Author

Ticket for Combine_Images script: https://trac.openmicroscopy.org.uk/ome/ticket/10960

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NB: Assuming objects is a list objects.sort(lambda left, right: cmp(left.id, right.id)) would suffice.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not trying to sort the objects by their own IDs. I'm trying to sort them so that they are in the same order as the list of ids that I used to retrieve them. E.g. ids = [5, 3, 4] then I want to sort objects to be: [<ImageWrapper id:5>, <ImageWrapper id:3>, <ImageWrapper id:4>]
I thought there might be a 'one-liner' to do it, but I couldn't work it out.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ugh. Ok. Agreed. As interesting as that sounds, I bet it's going to be way more confusing than your two liner! Sorry for the noise.

...objects.sort(lambda l,r: cmp(ids.index(l.id), ids.index(r.id)))
comes close but will throw a ValueError if the id isn't in ids
but so would yours.

@joshmoore
Copy link
Copy Markdown
Member

Being quiet now.

joshmoore added a commit that referenced this pull request May 24, 2013
@joshmoore joshmoore merged commit b55bc95 into ome:dev_4_4 May 24, 2013
@will-moore will-moore deleted the scripts_obj_sorting_10745 branch November 11, 2013 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants