-
Notifications
You must be signed in to change notification settings - Fork 2
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
Clean up routing behaviour in ngram component #1727
Changes from 7 commits
f3871ec
5c6c40f
63500bb
ca6f59f
792aabb
b410da3
cfaa837
4aa1f7e
47a0060
9e68ccf
66236de
6bf5203
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import { SimpleStore } from '../store/simple-store'; | ||
import { RouterStoreService } from '../store/router-store.service'; | ||
import { NgramParameters, NgramSettings } from './ngram'; | ||
|
||
describe('NgramParameters', ()=> { | ||
let store: RouterStoreService = new SimpleStore() as any; | ||
let ngramParameters: NgramParameters; | ||
const testState = { | ||
size: 3, | ||
positions: 'first', | ||
freqCompensation: true, | ||
analysis: 'clean', | ||
maxDocuments: 100, | ||
numberOfNgrams: 20, | ||
dateField: 'releaseDate' | ||
} as NgramSettings; | ||
const testParams = {ngramSettings: 's:3,p:first,c:true,a:clean,m:100,n:20,f:releaseDate'} | ||
|
||
beforeEach(() => { | ||
ngramParameters = new NgramParameters(store); | ||
}); | ||
|
||
it('should correctly convert internal state to a route parameter', () => { | ||
const params = ngramParameters.stateToStore(testState); | ||
expect(params).toEqual(testParams); | ||
}); | ||
|
||
it('should correctly convert a route parameter to an internal state', () => { | ||
const state = ngramParameters.storeToState(testParams); | ||
expect(state).toEqual(testState); | ||
}); | ||
|
||
it('should return default values if no relevant route parameter is present', () => { | ||
const defaultSettings = { | ||
size: 2, | ||
positions: 'any', | ||
freqCompensation: false, | ||
analysis: 'none', | ||
maxDocuments: 50, | ||
numberOfNgrams: 10, | ||
dateField: 'date' | ||
} as NgramSettings; | ||
const state = ngramParameters.storeToState({irrelevant: 'parameter'}) | ||
expect(state).toEqual(defaultSettings); | ||
}) | ||
|
||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
import { Params } from '@angular/router'; | ||
import * as _ from 'lodash'; | ||
|
||
import { StoreSync } from '../store/store-sync'; | ||
import { Store } from '../store/types'; | ||
|
||
export interface NgramSettings { | ||
size: number; | ||
positions: string; | ||
freqCompensation: boolean; | ||
analysis: string; | ||
maxDocuments: number; | ||
numberOfNgrams: number; | ||
dateField: string; | ||
} | ||
|
||
export class NgramParameters extends StoreSync<NgramSettings> { | ||
|
||
keysInStore = ['ngramSettings']; | ||
|
||
constructor(store: Store) { | ||
super(store); | ||
this.connectToStore(); | ||
} | ||
|
||
stateToStore(state: NgramSettings): Params { | ||
return { ngramSettings: [`s:${state.size}`,`p:${state.positions}`,`c:${state.freqCompensation}`, | ||
`a:${state.analysis}`,`m:${state.maxDocuments}`,`n:${state.numberOfNgrams}`, | ||
`f:${state.dateField}`].join(',') } | ||
} | ||
|
||
storeToState(params: Params): NgramSettings { | ||
if (_.has(params, 'ngramSettings')) { | ||
const stringComponents = params['ngramSettings'].split(','); | ||
return { | ||
size: parseInt(this.findSetting('s', stringComponents), 10), | ||
positions: this.findSetting('p', stringComponents), | ||
freqCompensation: this.findSetting('c', stringComponents) === 'true', | ||
analysis: this.findSetting('a', stringComponents), | ||
maxDocuments: parseInt(this.findSetting('m', stringComponents), 10), | ||
numberOfNgrams: parseInt(this.findSetting('n', stringComponents), 10), | ||
dateField: this.findSetting('f', stringComponents) | ||
} | ||
} | ||
return { | ||
size: 2, | ||
positions: 'any', | ||
freqCompensation: false, | ||
analysis: 'none', | ||
maxDocuments: 50, | ||
numberOfNgrams: 10, | ||
dateField: 'date' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way the date field is handled looks a bit shaky, since the corpus may not actually have a field with this name. I think this was already an issue, though. Have you checked that the visualisation still works with corpora like parliament-denmark-old? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, good point. The problem is that when the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I tried and abandoned the idea to initialize |
||
} as NgramSettings; | ||
} | ||
|
||
findSetting(abbreviation: string, stringComponents: string[]): string | undefined{ | ||
const setting = stringComponents.find(s => s[0] === abbreviation); | ||
return setting.split(':')[1]; | ||
} | ||
|
||
getCurrentRouterState(): string { | ||
return _.get(this.store.currentParams(), 'ngramSettings'); | ||
} | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only test in this spec was the one related to the ngram related models and types. |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change removes some unused imports, which I noticed when I was looking at other examples for testing routing behaviour.