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

Gestion des mots contractés #72

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/communeHelpers.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
const { initFields, initFormat } = require('./helpers');

const abbreviations = {
'st': 'saint',
'ste': 'sainte',
'cgne': 'campagne',
};
Copy link

@teleboas teleboas Feb 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A rapprocher de la liste utilisée pour les noms de rues dans la BAN ?
https://github.com/etalab/ban-data/blob/master/data/abbrev.txt



const initCommuneFields = initFields({
default: ['nom', 'code', 'codeDepartement', 'codeRegion', 'codesPostaux', 'population'],
base: ['nom', 'code'],
Expand All @@ -10,4 +17,4 @@ const initCommuneFormat = initFormat({
defaultGeometry: 'centre',
});

module.exports = { initCommuneFields, initCommuneFormat };
module.exports = { initCommuneFields, initCommuneFormat, abbreviations };
2 changes: 2 additions & 0 deletions lib/communes.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
const SearchableCollection = require('./searchableCollection');
const { abbreviations } = require('./communeHelpers.js');

const schema = {
nom: {
type: 'text',
queryWith: 'nom',
ref: 'code',
replacePatern: abbreviations,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replacePatterns

boost: {
population: (commune, score) => {
if (commune.population) {
Expand Down
5 changes: 5 additions & 0 deletions lib/searchableCollection/indexes/text.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const normalizeString = require('../normalizeString');
const replaceAbbreviations = require('../replaceAbbreviations');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Le nom me parait trop spécifique

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

À modifier

const lunr = require('lunr');
const { clone, sortBy } = require('lodash');

Expand All @@ -7,6 +8,7 @@ class TextIndex {
if (!key) throw new Error('key is required');
this._key = key;
this._boost = options.boost || {};
this._replacePatern = options.replacePatern || {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_replacePatterns

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pas utile de mettre une valeur par défaut coûteuse à tester

const refKey = this._refKey = options.ref || 'id';
this._refIndex = new Map();
this._index = lunr(function () {
Expand All @@ -31,6 +33,9 @@ class TextIndex {

find(terms, options = {}) {
let boosted = false;

terms = replaceAbbreviations(terms, this._replacePatern);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (this._replacePatterns) {
  terms = replace(terms, this._replacePatterns);
}


const results = this._index.search(terms)
.map(result => {
const item = clone(this._refIndex.get(result.ref));
Expand Down
17 changes: 17 additions & 0 deletions lib/searchableCollection/replaceAbbreviations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const removeDiacritics = require('./removeDiacritics');

function replaceAbbreviations(search, abbreviations) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

terms, patterns

const terms = removeDiacritics(search)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi utilise removeDiacritics ici ?

.toLowerCase()
.replace(/"/g, '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un commentaire est bienvenu

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je pense que tu fais trop de chose dans cette fonction replace. Mieux faut avoir plusieurs fonctions, les appeler dans le bon ordre et leur donner le nom adapté.

.replace(/-/g, ' ')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idem

.split(' ');

if (terms.length <= 1) return search;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

search est trop abstrait

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Au lieu de faire toutes ces transformations pour rien, je ferai le test de la présence d'un espace dès le début.

On fait de l'auto-complétion, chaque miliseconde compte.


return terms
.map(token => token in abbreviations ? abbreviations[token] : token)
.join(' ');
}

module.exports = replaceAbbreviations;
74 changes: 74 additions & 0 deletions test/replaceAbbreviations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/* eslint-env mocha */
const expect = require('expect.js');
const replaceAbbreviations = require('../lib/searchableCollection/replaceAbbreviations');

describe.only('replaceAbbreviations()', function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only

const abbreviations = {
'st': 'saint',
'ste': 'sainte',
'cgne': 'campagne',
};

beforeEach(done => {
done();
});

describe('Words separated by spaces', function () {
it('should replace patern', function () {
const str = '"st louis"';
const out = 'saint louis';

expect(replaceAbbreviations(str, abbreviations)).to.equal(out);
});

it('should replace patern', function () {
const str = '"marcilly la cgne"';
const out = 'marcilly la campagne';

expect(replaceAbbreviations(str, abbreviations)).to.equal(out);
});
});

describe('Words separated by dashes', function () {
it('should replace patern', function () {
const str = '"st-louis"';
const out = 'saint louis';

expect(replaceAbbreviations(str, abbreviations)).to.equal(out);
});

it('should replace patern', function () {
const str = '"marcilly-la-cgne"';
const out = 'marcilly la campagne';

expect(replaceAbbreviations(str, abbreviations)).to.equal(out);
});
});

describe('search contained only one word', () => {
it('should not replace patern', () => {
const str = '"st"';

expect(replaceAbbreviations(str, abbreviations)).to.equal(str);
});
});

describe('Patern is contained in a word', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pattern

it('should not replace patern', () => {
const str = '"le stinx"';
const out = 'le stinx';

expect(replaceAbbreviations(str, abbreviations)).to.equal(out);
});
});

describe('Accents management', () => {
it('sould not replace the accent with a space', () => {
const str = '"St-Auban-sur-l\'Ouvèze"';
const out = 'saint auban sur l\'ouveze';

expect(replaceAbbreviations(str, abbreviations)).to.equal(out);
});
});

});