Skip to content

Feature: cuboids-from-id#79

Open
dxenes1 wants to merge 7 commits intomasterfrom
cuboids_from_id
Open

Feature: cuboids-from-id#79
dxenes1 wants to merge 7 commits intomasterfrom
cuboids_from_id

Conversation

@dxenes1
Copy link
Copy Markdown
Contributor

@dxenes1 dxenes1 commented Mar 26, 2021

Adds remote function for the cuboidsfromid api in bossDB. It is now possible to get a list of cuboids that belong to a specific annotation ID. Remote syntax is as follows:

rmt = BossRemote()
resource = rmt.get_channel('foo', 'bar', 'baz')
rmt.get_cuboids_from_id(resource, resolution=0, id='18203028421')
>> {'cuboids': [[512, 512, 64],
  [0, 512, 32],
  [512, 512, 32],
  [0, 512, 48],
  [512, 512, 48]]}

@dxenes1 dxenes1 self-assigned this Mar 26, 2021
Copy link
Copy Markdown
Member

@movestill movestill left a comment

Choose a reason for hiding this comment

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

f string comment is an optional change. Please specify which corner of the cuboids are returned in the docstring.

if url_prefix is None or url_prefix == '':
raise RuntimeError('url_prefix required.')

url = (url_prefix + '/' + self.version + '/cuboidsfromid/' +
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.

Now that we're only supporting modern Python versions 🥳, you could switch to an f string which I think would look nicer.

def get_cuboids_from_id(self, resource, resolution, id):
"""Get corners for cuboids that belong to a specific ID.

All returned corners are cuboid aligned.
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.

I think we should say which corner is returned. IIRC, it's the corner closest to the origin?

id (int): Id of object of interest.

Returns:
(dict): {'cuboids': [[512, 512, 64], [0, 512, 32], [512, 512, 32], [0, 512, 48]]}
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.

Looks good, just update the returns comment to match the new comments for get_cuboids_from_id().

@j6k4m8
Copy link
Copy Markdown
Member

j6k4m8 commented Jun 29, 2022

@dxenes1 can we merge this?

@dxenes1
Copy link
Copy Markdown
Contributor Author

dxenes1 commented Jun 30, 2022

@dxenes1 can we merge this?

Still waiting for the changes to be merged in BossDB backend, but after that it should be good

@dxenes1 dxenes1 marked this pull request as ready for review August 2, 2022 17:00
@dxenes1
Copy link
Copy Markdown
Contributor Author

dxenes1 commented Aug 2, 2022

Updated docs and published this PR as ready for review (and merge)

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.

3 participants