From 6732185d155222bcba64ef67e1930a452a0492e3 Mon Sep 17 00:00:00 2001 From: Xiao Gui <xgui3783@gmail.com> Date: Tue, 20 Jul 2021 16:51:01 +0200 Subject: [PATCH] bugfix: change tmpl spatial backend --- .../parcellationRegion/region.base.spec.ts | 138 ++++++++++++------ .../parcellationRegion/region.base.ts | 26 ++-- ...eCoordinatesTransformation.service.spec.ts | 47 ++++-- ...mplateCoordinatesTransformation.service.ts | 24 ++- 4 files changed, 164 insertions(+), 71 deletions(-) diff --git a/src/atlasComponents/parcellationRegion/region.base.spec.ts b/src/atlasComponents/parcellationRegion/region.base.spec.ts index 0fb3792e9..c9ee29648 100644 --- a/src/atlasComponents/parcellationRegion/region.base.spec.ts +++ b/src/atlasComponents/parcellationRegion/region.base.spec.ts @@ -1,6 +1,6 @@ import { TestBed } from '@angular/core/testing' import { MockStore, provideMockStore } from '@ngrx/store/testing' -import { viewerStateNewViewer } from 'src/services/state/viewerState/actions' +import { viewerStateSelectTemplateWithId } from 'src/services/state/viewerState/actions' import { RegionBase, regionInOtherTemplateSelector, getRegionParentParcRefSpace } from './region.base' const util = require('common/util') @@ -620,16 +620,18 @@ describe('> region.base.ts', () => { }) describe('> changeView', () => { const fakeTmpl = { + '@id': 'faketmplid', name: 'fakeTmpl' } const fakeParc = { + '@id': 'fakeparcid', name: 'fakeParc' } beforeEach(() => { regionBase = new RegionBase(mockStore) }) - describe('> if sameRegion has position attribute', () => { + describe('> [tmp] sameRegion to use transform backend', () => { let dispatchSpy: jasmine.Spy beforeEach(() => { @@ -638,64 +640,106 @@ describe('> region.base.ts', () => { afterEach(() => { dispatchSpy.calls.reset() }) - it('> malformed position is not an array > do not pass position', () => { - regionBase.changeView({ - template: fakeTmpl, - parcellation: fakeParc, - region: { - position: 'hello wolrd' - } - }) - - expect(dispatchSpy).toHaveBeenCalledWith( - viewerStateNewViewer({ - selectTemplate: fakeTmpl, - selectParcellation: fakeParc, - navigation: {} - }) - ) - }) - - it('> malformed position is an array of incorrect size > do not pass position', () => { + it('> calls viewerStateSelectTemplateWithId', () => { regionBase.changeView({ template: fakeTmpl, parcellation: fakeParc, - region: { - position: [] - } }) expect(dispatchSpy).toHaveBeenCalledWith( - viewerStateNewViewer({ - selectTemplate: fakeTmpl, - selectParcellation: fakeParc, - navigation: {} - }) - ) - }) - - it('> correct position > pass position', () => { - regionBase.changeView({ - template: fakeTmpl, - parcellation: fakeParc, - region: { - position: [1,2,3] - } - }) - - expect(dispatchSpy).toHaveBeenCalledWith( - viewerStateNewViewer({ - selectTemplate: fakeTmpl, - selectParcellation: fakeParc, - navigation: { - position: [1,2,3] + viewerStateSelectTemplateWithId({ + payload: { + '@id': fakeTmpl['@id'] + }, + config: { + selectParcellation: { + "@id": fakeParc['@id'] + } } }) ) }) }) + + /** + * currently, without position metadata, the navigation is broken + * fix changeView to fetch position metadata. If cannot, fallback to spatial backend + */ + + // describe('> if sameRegion has position attribute', () => { + // let dispatchSpy: jasmine.Spy + + // beforeEach(() => { + // dispatchSpy = spyOn(mockStore, 'dispatch') + // }) + // afterEach(() => { + // dispatchSpy.calls.reset() + // }) + // it('> malformed position is not an array > do not pass position', () => { + + // regionBase.changeView({ + // template: fakeTmpl, + // parcellation: fakeParc, + // region: { + // position: 'hello wolrd' + // } + // }) + + // expect(dispatchSpy).toHaveBeenCalledWith( + // viewerStateNewViewer({ + // selectTemplate: fakeTmpl, + // selectParcellation: fakeParc, + // navigation: {} + // }) + // ) + // }) + + // it('> malformed position is an array of incorrect size > do not pass position', () => { + + // regionBase.changeView({ + // template: fakeTmpl, + // parcellation: fakeParc, + // region: { + // position: [] + // } + // }) + + // expect(dispatchSpy).toHaveBeenCalledWith( + // viewerStateSelectTemplateWithId({ + // payload: { + // '@id': fakeTmpl['@id'] + // }, + // config: { + // selectParcellation: { + // "@id": fakeParc['@id'] + // } + // } + // }) + // ) + // }) + + // it('> correct position > pass position', () => { + // regionBase.changeView({ + // template: fakeTmpl, + // parcellation: fakeParc, + // region: { + // position: [1,2,3] + // } + // }) + + // expect(dispatchSpy).toHaveBeenCalledWith( + // viewerStateNewViewer({ + // selectTemplate: fakeTmpl, + // selectParcellation: fakeParc, + // navigation: { + // position: [1,2,3] + // } + // }) + // ) + // }) + // }) }) }) }) diff --git a/src/atlasComponents/parcellationRegion/region.base.ts b/src/atlasComponents/parcellationRegion/region.base.ts index d954d08b1..c91ef45d1 100644 --- a/src/atlasComponents/parcellationRegion/region.base.ts +++ b/src/atlasComponents/parcellationRegion/region.base.ts @@ -1,10 +1,10 @@ import { EventEmitter, Input, Output, Pipe, PipeTransform } from "@angular/core"; import { select, Store, createSelector } from "@ngrx/store"; import { uiStateOpenSidePanel, uiStateExpandSidePanel, uiActionShowSidePanelConnectivity } from 'src/services/state/uiState.store.helper' -import { distinctUntilChanged, switchMap, filter, map, withLatestFrom } from "rxjs/operators"; +import { distinctUntilChanged, switchMap, filter, map, withLatestFrom, tap } from "rxjs/operators"; import { Observable, BehaviorSubject, combineLatest } from "rxjs"; import { flattenRegions, getIdFromKgIdObj, rgbToHsl } from 'common/util' -import { viewerStateSetConnectivityRegion, viewerStateNavigateToRegion, viewerStateToggleRegionSelect, viewerStateNewViewer, isNewerThan } from "src/services/state/viewerState.store.helper"; +import { viewerStateSetConnectivityRegion, viewerStateNavigateToRegion, viewerStateToggleRegionSelect, viewerStateNewViewer, isNewerThan, viewerStateSelectTemplateWithId } from "src/services/state/viewerState.store.helper"; import { viewerStateFetchedTemplatesSelector, viewerStateGetSelectedAtlas, viewerStateSelectedTemplateFullInfoSelector, viewerStateSelectedTemplateSelector } from "src/services/state/viewerState/selectors"; import { strToRgb, verifyPositionArg, getRegionHemisphere } from 'common/util' import { getPosFromRegion } from "src/util/siibraApiConstants/fn"; @@ -181,23 +181,25 @@ export class RegionBase { const { template, parcellation, - region } = sameRegion - const { position } = region - const navigation = Array.isArray(position) && position.length === 3 - ? { position } - : { } this.closeRegionMenu.emit() /** * TODO use createAction in future * for now, not importing const because it breaks tests */ - this.store$.dispatch(viewerStateNewViewer ({ - selectTemplate: template, - selectParcellation: parcellation, - navigation, - })) + this.store$.dispatch( + viewerStateSelectTemplateWithId({ + payload: { + '@id': template['@id'] || template['fullId'] + }, + config: { + selectParcellation: { + '@id': parcellation['@id'] || parcellation['fullId'] + } + } + }) + ) } } diff --git a/src/services/templateCoordinatesTransformation.service.spec.ts b/src/services/templateCoordinatesTransformation.service.spec.ts index 08906f068..0aa477d10 100644 --- a/src/services/templateCoordinatesTransformation.service.spec.ts +++ b/src/services/templateCoordinatesTransformation.service.spec.ts @@ -33,8 +33,8 @@ describe('templateCoordinatesTransformation.service.spec.ts', () => { // subscriptions are necessary for http fetch to occur service.getPointCoordinatesForTemplate( - 'from', - 'target', + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.MNI152, + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.BIG_BRAIN, [1,2,3] ).subscribe((_ev) => { @@ -44,8 +44,8 @@ describe('templateCoordinatesTransformation.service.spec.ts', () => { expect( JSON.parse(req.request.body) ).toEqual({ - 'source_space': 'from', - 'target_space': 'target', + 'source_space': TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.MNI152, + 'target_space': TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.BIG_BRAIN, 'source_points': [ [1e-6, 2e-6, 3e-6] ] @@ -53,14 +53,41 @@ describe('templateCoordinatesTransformation.service.spec.ts', () => { req.flush({}) }) + it('transforms mapped space name(s)', () => { + const service = TestBed.inject(TemplateCoordinatesTransformation) + const httpTestingController = TestBed.inject(HttpTestingController) + + const key = Array.from(TemplateCoordinatesTransformation.NameMap.keys())[0] + service.getPointCoordinatesForTemplate( + key, + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.BIG_BRAIN, + [1,2,3] + ).subscribe((_ev) => { + + }) + const req = httpTestingController.expectOne(service.url) + expect(req.request.method).toEqual('POST') + expect( + JSON.parse(req.request.body) + ).toEqual({ + 'source_space': TemplateCoordinatesTransformation.NameMap.get(key), + 'target_space': TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.BIG_BRAIN, + 'source_points': [ + [1e-6, 2e-6, 3e-6] + ] + }) + req.flush({}) + }) + + it('should transform response properly', () => { const service = TestBed.inject(TemplateCoordinatesTransformation) const httpTestingController = TestBed.inject(HttpTestingController) service.getPointCoordinatesForTemplate( - 'from', - 'target', + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.MNI152, + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.BIG_BRAIN, [1,2,3] ).subscribe(({ status, result }) => { expect(status).toEqual('completed') @@ -79,8 +106,8 @@ describe('templateCoordinatesTransformation.service.spec.ts', () => { const httpTestingController = TestBed.inject(HttpTestingController) service.getPointCoordinatesForTemplate( - 'from', - 'target', + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.MNI152, + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.BIG_BRAIN, [1,2,3] ).subscribe(({ status }) => { expect(status).toEqual('error') @@ -96,8 +123,8 @@ describe('templateCoordinatesTransformation.service.spec.ts', () => { const httpTestingController = TestBed.inject(HttpTestingController) service.getPointCoordinatesForTemplate( - 'from', - 'target', + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.MNI152, + TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.BIG_BRAIN, [1,2,3] ).subscribe(({ status, statusText }) => { expect(status).toEqual('error') diff --git a/src/services/templateCoordinatesTransformation.service.ts b/src/services/templateCoordinatesTransformation.service.ts index 6aaf98b99..2c6f4fce9 100644 --- a/src/services/templateCoordinatesTransformation.service.ts +++ b/src/services/templateCoordinatesTransformation.service.ts @@ -14,6 +14,18 @@ export interface ITemplateCoordXformResp{ }) export class TemplateCoordinatesTransformation { + static VALID_TEMPLATE_SPACE_NAMES = { + MNI152: 'MNI 152 ICBM 2009c Nonlinear Asymmetric', + COLIN27: 'MNI Colin 27', + BIG_BRAIN: 'Big Brain (Histology)', + INFANT: 'Infant Atlas', + } + + static NameMap = new Map([ + ['MNI152 2009c nonl asym', TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.MNI152], + ["Big Brain", TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES.BIG_BRAIN] + ]) + constructor(private httpClient: HttpClient) {} public url = `${SPATIAL_TRANSFORM_BACKEND.replace(/\/$/, '')}/v1/transform-points` @@ -27,12 +39,20 @@ export class TemplateCoordinatesTransformation { 'Content-Type': 'application/json' }) } + const srcTmplName = Object.values(TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES).includes(sourceTemplateName) + ? sourceTemplateName + : TemplateCoordinatesTransformation.NameMap.get(sourceTemplateName) + + const targetTmplName = Object.values(TemplateCoordinatesTransformation.VALID_TEMPLATE_SPACE_NAMES).includes(targetTemplateName) + ? targetTemplateName + : TemplateCoordinatesTransformation.NameMap.get(targetTemplateName) + return this.httpClient.post( this.url, JSON.stringify({ 'source_points': [[...coordinatesInNm.map(c => c/1e6)]], - 'source_space': sourceTemplateName, - 'target_space': targetTemplateName + 'source_space': srcTmplName, + 'target_space': targetTmplName }), httpOptions ).pipe( -- GitLab