-
Notifications
You must be signed in to change notification settings - Fork 16
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
"Select all studies" always creates a new analysis #93
Comments
One possibility I lean towards would be to just drop the selection interface everywhere else on the site except the custom analysis interface, and add the facility to search by location to the latter (since it's just a call to an API route that already exists). That's the only thing you can't currently do from the custom interface, I think. Let me mull it over... |
@tyarkoni The current behavior is definitely wrong, because it starts a new headless (i.e. untitled) analysis but the selection doesn't actually show up in the custom analysis interface. I'll fix it so at least the selection in the location interface actually carries over to the custom analysis interface (as a new analysis). Longer term, I think it makes sense to unify all of the different ways of selecting studies into the same interface. Ideally the same API route would take care of filtering based on expression, location or both. This should be fairly easy to do on the backend, i.e. something like:
We could add 4 basic input boxes for the 3 coordinates plus the radius next to the expression input field, e.g.:
The user would still use the existing location page to explore and decide on a location and radius. Then they would enter that location and radius into the custom analysis interface if they want to filter by location. |
I like that, let's go with your proposal (when you next have time to work on it). The backend change will be super easy because the underlying Neurosynth code actually already allows multiple search criteria. Everything goes through a get_studies() call, so you can do something like: dataset.get_studies(expression="language", peaks=[[0, 2, 0]], r=6) ...which would do exactly what your example does. The change to the celery task would be pretty trivial. Let me know when you get back to this and I'll make the back-end change (or you can give it a shot if you like). I have a grant proposal due in 2 weeks so I'm probably not going to touch this codebase much if at all between now and then. |
Sounds good. I can care of both the front-end and backend changes. |
Not entirely sure how to handle this, but right now, if a user's on a page other than the custom analysis interface, and clicks the "select all studies on this page" button, a new analysis is always created. This isn't necessarily desirable, since it makes it impossible to combine studies selected using different criteria (e.g., all studies that activate within 8 mm of a particular locations, plus all studies that use the term 'emotion').
It's a bit tricky, because I don't think we want to always add studies to the currently loaded analysis. But I also don't think we want to overwhelm the user by popping up boxes all over the site asking things like "do you want to add these studies to the current analysis, or create a new one?". Let me know if you have suggestions for how to implement this, but no need to change anything for now.
The text was updated successfully, but these errors were encountered: