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

TVB-2757: Apply filters on linked datatypes at runtime by rerendering the form #351

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

VinczeRobert
Copy link
Member

@VinczeRobert VinczeRobert commented Jan 20, 2021

At the beginning we thought this method might be simpler than what I did on the other PR of this task, but now I am not so sure about that. I think that at least the code is easier to understand.

I chose to separate the 3 types of filters (default, user defined and runtime filters) in 3 dictionaries because they need to be handled differently:

  • default filters just need to stay hidden in their respective select field elements
  • user defined filters need to not stay hidden and they need to be readded after the form is rerendered
  • runtime filters also need to stay hidden but the values of those filters need to be replaced in the Python code in cases such as when you filter the connectivities based on the connectivity annotations (I call this 'reverse_filtering' because you don't have an 'fk_connectivity_annotations field on ConnectivityIndex, so I used ConnectivityIndex.number_of_regions comparing it with the number of regions of the connectivity with the gid equal to ConnectivityAnnotationsIndex.fk_connectivity_gid).

Anothe problem was that after you rerender the form, the most recently added datatypes will appear to be the selected values by default, so I needed to keep the values before applying the filters to reput them after the form is rerendered.

I have two bugs (aspects I still have to take care of) currently:

  1. When reading the filters I am using the dynamic filters of the field that currently had its value changed but I should use the dynamic filters of each field when readding the filters for each particular field. (EDIT: I solved this one)
  2. I didn't take care of the reverse filtering in the get_filtered_datatypes method_yet. (EDIT: I solved this one)

@VinczeRobert
Copy link
Member Author

I solved the two issues I described above, but I found two new ones regarding the surface/region mapping viewers.
I didn't know that basically the surface viewer is a subclass of the region mapping viewer. The problem with that is the need for filtering region mappings based on the currently chosen surface inside the surface viewer. That goes well, but when you go to the region mapping viewer, the same filter will be applied, but there is not even a base surface on that page (I don't mean the shell surface) so that field will always be None. I know there are some common fields there, but couldn't we separate the two viewers? Also, there is another small reason that makes me think we should separate them: the region mapping field is not required in the surface viewer, but it should be required in the region mapping viewer...

The second issue is that in the surface viewer we should filter the connectivity measures based on the chosen region mapping. The region_mapping_index.fk_connectivity_gid and connectivity_measure_index.fk_connectivity_gid connectivities should have the the same number of regions. Currently my solution does not cover this case, because there is no connectivity field in the UI of that page, so I need to extend it a little to cover this case as well. (A simple but probably ugly solution would be to keep a hidden connectivity field on that page)

@VinczeRobert VinczeRobert changed the title WIP TVB-2757: Apply filters on linked datatypes at runtime by rerendering the form TVB-2757: Apply filters on linked datatypes at runtime by rerendering the form Jan 22, 2021
@VinczeRobert
Copy link
Member Author

There is one situation I haven't added filters but I could have and that is for structuring MRIs. There are some Region Volume Mappings and Time Series Volumes that don't go with certain Structural MRIs and I wanted to filter them to match the fk_volume_gid, but I personally have no data that would fulfill that criteria so I chose to just let it be. Maybe we can do some filtering based on some other data, but that would just complicate even more the solution for this task.

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.

1 participant