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

Route to find data #3682

Merged
merged 18 commits into from
Feb 21, 2019
Merged

Route to find data #3682

merged 18 commits into from
Feb 21, 2019

Conversation

youri-k
Copy link
Contributor

@youri-k youri-k commented Jan 24, 2019

  • backend part is finished, new datastore route: GET http://localhost:9000/data/datasets/Connectomics_Department/ROI2017_wkw/layers/color/findData?token=secretScmBoyToken

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • click icon in dataset view tab
  • if dataset contains data a position with data should be shown

Issues:


@youri-k youri-k self-assigned this Jan 24, 2019
)
for {
(data, _) <- requestData(dataSource, dataLayer, request)
if data.nonEmpty
Copy link
Member

Choose a reason for hiding this comment

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

This syntax is new to me, can we get rid of bool2Fox this way?
However, nonEmpty does not suffice, since the array is still nonEmpty if it contains 32768 zeroes. And indeed, the route always returns the center of the bounding box for me

@philippotto philippotto self-assigned this Jan 30, 2019
@philippotto
Copy link
Member

Front-end part is finished. I added a subtle icon next to the layers in the dataset settings. Strictly speaking, it's not a setting, but I didn't want to clutter our menus with more random stuff. The tooltip text also might need iteration. Feel free to leave feedback!

For the record, @youri-k is going to push another commit to improve the actual find-data route.

@youri-k
Copy link
Contributor Author

youri-k commented Jan 31, 2019

Sadly the code isn't working right now and I don't know why. One culprit is the HashSet that does not save values through iterations. Maybe @fm3 you have an idea, what to do next?

@fm3
Copy link
Member

fm3 commented Feb 4, 2019

@youri-k can you test if the hashset has this problem? How is it passed between iterations / how is ensured that they all use the same?
Also, as I understand, the HashSet is only used to prevent unnecessary extra work, right? Does this kill the feature? If it just takes a second longer without it, I’d say it’s fine in the first version.

@youri-k
Copy link
Contributor Author

youri-k commented Feb 14, 2019

I changed the implementation back to the original idea, as I figured out that it didn't really matter.
@philippotto I think my merge attempt broke the frontend so it would be nice if you can take a look again.

@youri-k youri-k changed the title [WIP] Route to find data Route to find data Feb 21, 2019
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

works for me + backend LGTM

@fm3 fm3 merged commit 0906653 into master Feb 21, 2019
@fm3 fm3 deleted the find-data-route branch February 21, 2019 14:23
@@ -13,6 +13,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).

- Added the possibility to create volume annotation tasks. When creating a task type, select whether to create `volume` or `skeleton` tasks. Note that compound viewing for volume tasks is not supported yet. Same for creating volume tasks from uploaded nml/data files. [#3712](https://github.com/scalableminds/webknossos/pull/3712)
- Added an UI to select a mapping for a segmentation layer. The UI is placed in the segmentation tab within the tracing view. [#3720](https://github.com/scalableminds/webknossos/pull/3720)
- Added a button to jump to actual data if the bounding box of a dataset contains a lot of black data. [#3682](https://github.com/scalableminds/webknossos/pull/3720)
Copy link
Contributor

Choose a reason for hiding this comment

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

fixed the link here: #3821

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide backend route to "find data"
4 participants