feat: use synonyms cvs file to generate extra suggest records #17
Conversation
| if err != nil { | ||
| return nil, err | ||
| } | ||
| var fieldValuesByNameExtensions = make(map[string][]string) | ||
| synonyms, err := readSubstitutionsFile(synonymsFile) |
There was a problem hiding this comment.
Zou hier readCsvFile van maken aangezien het voor zowel subst als synonyms wordt gebruikt
| } | ||
| } | ||
| } | ||
|
|
||
| // Todo: avoid getting duplicates (for performance improvement) |
There was a problem hiding this comment.
Het is meer bedoelt remark. Ik verwacht geen performance issues, maar in theorie kan het efficienter.
| @@ -134,3 +163,16 @@ func readSubstitutionsFile(filepath string) (map[string]string, error) { | |||
| } | |||
| return substitutions, err | |||
| } | |||
|
|
|||
| func reverseMap(m map[string]string) (map[string]string, error) { | |||
There was a problem hiding this comment.
In gomapgpie (en gokoala want daar komt het uit) is al een util.Inverse() function beschikbaar om een map om te draaien. Misschien kan je die gebruiken? Zo niet dan geen big deal, maar zou mooi zijn als het kan.
There was a problem hiding this comment.
Die had ik niet gezien, super! DONE
| if !tt.wantErr(t, err, fmt.Sprintf("generateAllFieldValues(%v, %v, %v)", tt.args.fieldValuesByName, tt.args.substitutionsFile, tt.args.synonymsFile)) { | ||
| return | ||
| } | ||
| assert.Equalf(t, tt.want, got, "generateAllFieldValues(%v, %v, %v)", tt.args.fieldValuesByName, tt.args.substitutionsFile, tt.args.synonymsFile) |
There was a problem hiding this comment.
Gebruik eigenlijk altijd assert.Equal ipv assert.Equalf, is er een reden om de formatting variant te gebruiken?
| valueLower := strings.ToLower(value.(string)) | ||
|
|
||
| // Get all substitutions | ||
| substitutedValues, err := extentValues([]string{valueLower}, substitutions) |
There was a problem hiding this comment.
extent vs extend (filename is extend). Moet extend zijn volgens mij (snap de extent wel gezien de geo werkzaamheden 😉 )
There was a problem hiding this comment.
:) deze zat op meer plekken. DONE
| func generateFieldValuesSubstitutions(fieldValuesByName map[string]any, substitutionFile string) ([]map[string]any, error) { | ||
| substitutions, err := readSubstitutionsFile(substitutionFile) | ||
| // Return slice of fieldValuesByName | ||
| func generateAllFieldValues(fieldValuesByName map[string]any, substitutionsFile, synonymsFile string) ([]map[string]any, error) { |
There was a problem hiding this comment.
Misschien zou je dit extendFieldValues kunnen noemen? Dan sluit het wat meer aan bij de filename.
| } | ||
| return generateAllFieldValuesByName(fieldValuesByNameExtensions), err | ||
| return generateAllFieldValuesByName(fieldValuesByNameWithAllValues), err | ||
| } | ||
|
|
||
| // Transform a map[string][]string into a []map[string]string using the cartesian product, i.e. |
There was a problem hiding this comment.
Zit niet in deze change maar een recursive functie kan je bijna altijd ook zonder recursie schrijven. generateCombinations is recursief, heb een poging gedaan deze zonder recursie op te lossen:
func generateCombinations(keys []string, values [][]string) []map[string]any {
if len(keys) == 0 || len(values) == 0 {
return nil
}
result := []map[string]any{{}} // contains empty map so the first iteration works
for keyDepth := 0; keyDepth < len(keys); keyDepth++ {
var newResult []map[string]any
for _, entry := range result {
for _, val := range values[keyDepth] {
newEntry := make(map[string]any)
for k, v := range entry {
newEntry[k] = v
}
newEntry[keys[keyDepth]] = val
newResult = append(newResult, newEntry)
}
}
result = newResult
}
return result
}
En de test:
func Test_generateCombinations(t *testing.T) {
type args struct {
keys []string
values [][]string
}
tests := []struct {
name string
args args
want []map[string]any
}{
{"Single key, single value", args{[]string{"key1"}, [][]string{{"value1"}}}, []map[string]any{{"key1": "value1"}}},
{"Single key, slice of values", args{[]string{"key1"}, [][]string{{"value1", "value2"}}}, []map[string]any{{"key1": "value1"}, {"key1": "value2"}}},
{"Two keys, two single values", args{[]string{"key1", "key2"}, [][]string{{"value1"}, {"value2"}}}, []map[string]any{{"key1": "value1", "key2": "value2"}}},
{"Two keys, slice + single value", args{[]string{"key1", "key2"}, [][]string{{"value1", "value2"}, {"value3"}}}, []map[string]any{{"key1": "value1", "key2": "value3"}, {"key1": "value2", "key2": "value3"}}},
{"Two keys, two slices values", args{[]string{"key1", "key2"}, [][]string{{"value1", "value2"}, {"value3", "value4"}}}, []map[string]any{{"key1": "value1", "key2": "value3"}, {"key1": "value1", "key2": "value4"}, {"key1": "value2", "key2": "value3"}, {"key1": "value2", "key2": "value4"}}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, generateCombinations(tt.args.keys, tt.args.values), "generateCombinations(%v, %v)", tt.args.keys, tt.args.values)
})
}
}
There was a problem hiding this comment.
Mooi en clean! Heb hem overgenomen
| @@ -133,3 +161,28 @@ func Test_readSubstitutions(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| func Test_reverseMap1(t *testing.T) { | |||
There was a problem hiding this comment.
niet meer relevant nu ik de util.Inverse() gebruik
kad-dirc
force-pushed
the
PDOK-17192/synoniem-subst
branch
from
849aa9a to
9ef1454
Compare
Description
use synonyms cvs file to generate extra suggest records
Type of change
Checklist: