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

fix(person): change fullName to use name patterns #1637

Merged
merged 28 commits into from
Feb 2, 2023

Conversation

matthewmayer
Copy link
Contributor

@matthewmayer matthewmayer commented Dec 6, 2022

Fixes #1211

This is work-in-progress tentative draft fix for #1211 which is a simplified version of the ideas in #1216.

  • Allows the use of the src/locales/*/person/name.ts legacy patterns
  • Allows for locales where first name and last name are reversed e.g. ja, zh_CN
  • Allows for prefixes and suffixes in some languages
  • Removes gender-specific templates. In particular you don't need to repeat male and female patterns in name.ts any more so I simplified all locales where that was the case
  • Since src/locales/en/person/name.ts is defined, don't need a final fallback, we can assume there will always be a name pattern
  • Uses the new weightedArrayElement to replicate the old behavior of fullName where prefixes and suffixes are only addded to a small proportion of names feat(helpers): add new faker.helpers.weightedArrayElement #1654

Sample output: en, generic

'Ramiro Wehner',      'Ernest Smith',
  'Kristopher Gutmann', 'Jason Price',
  'Shannon Kuhn',       'Camille Schoen',
  'Israel Kreiger',     'Mrs. Diane Spinka MD',
  'Jon Shields',        'Luke Boyer',
  'Gerard Parker',      'Mrs. Amber DuBuque IV',
  'Ms. Flora Keebler',  'Orlando Cummerata',
  'Ross Friesen MD',    'Miss Sonya Kuhlman V',
  'Estelle DuBuque',    'Malcolm Weber PhD',
  'Mr. Juan Hamill',    'Ms. Lora Rempel MD'

Sample output: en, male

'Morris Cummerata I',
  'Jesse Wolff',
  'Harry Erdman',
  'Rudy Swift',
  'Dwight Feest',
  'Nathan Schumm',
  'Roosevelt Kuhn MD',
  'Shawn Stokes I',
  'Mr. Dexter Wintheiser IV',
  'Leonard Marks Jr.',
  'Lloyd Von',
  'Jon Frami',
  'Mr. Jody Sipes MD',
  'Micheal Ward',
  'Alberto Feest',
  'Terrance Schuppe',
  'Leroy Feest IV',
  'Mr. Duane Senger I',
  'Ismael Walsh',
  'Claude Casper'

Sample output: en, female

 'Patty West',            'Hope Lueilwitz',
  'Gwen Skiles',           'Anita Homenick',
  "Lucia O'Reilly DVM",    'Dr. Ashley Aufderhar',
  'Karla Keeling',         'Sonia Corwin',
  'Kathryn Lang',          'Velma Mertz',
  'Raquel Funk II',        'Amanda Ebert',
  'Mrs. Claire Corwin II', 'Penny Reichel',
  'Ms. Rachael Hermann V', 'Adrienne Roob',
  'Alice Fisher',          'Patty Stehr',
  'Sally Lesch',           'Dianna Kohler'

Sample output: es

'Mrs. Jennifer Alanis Pineda',
  'Miss Ana Saavedra Muñiz',
  'Ms. Marta Moya Valencia',
  'Dr. María de los Ángeles Ledesma Osorio',
  'Ángel Solís Orta',
  'Lorena Munguía Rivera',
  'Laura Meza Moya',
  'Laura Alonso Orozco',
  'Mr. Pablo Mora Pichardo',
  'Daniel Pizarro Rosado',
  'Ana Salcedo de Jesús',
  'Mr. Manuel Rascón Leyva',
  'Iván Delarosa Bustos',
  'Mrs. Andrea Rael Navarro',
  'Dr. Jennifer Arce Garrido',
  'Dr. Daniel Rael Arce',
  'Mrs. Jennifer Marrero Caraballo',
  'Sergi Carrero Valladares',
  'Dr. Pablo Jaimes Espinoza',
  'Rubén Chávez Lira'

Sample output: zh_CN (note lastname before firstname)

'郭语汐', '邵宇轩', '韩丹',
  '洪国栋', '阎俊熙', '曾国良',
  '梁丽萍', '秦浩然', '胡萍',
  '任国英', '熊雨桐', '郭梓涵',
  '崔婷',   '莫兰英', '程依诺',
  '罗国香', '陶国英', '卢辉',
  '邓志明', '夏洁'

All locales

(note there are still first name issues in some languages due to #1610 (comment) )

Object.keys(faker.locales).forEach((locale) => {
  faker.setLocale(locale);
  console.log(
    `${locale}: (${
      faker.locales[locale].person?.name?.length || 0
    }) ${faker.person.fullName()},${faker.person.fullName({
      sex: 'male',
    })},${faker.person.fullName({ sex: 'female' })}`
  );
});

Number of name patterns is shown in paretheses

af_ZA: (0) Eugene van Staden,Mr. Bobbie Krige,Elsa Swart
ar: (3) Mr. عمار سويس,Mr. ريان فقوسة,جاوحدو خلود
az: (3) Vəsiyeva Elyanora,Quliyev Nadir,Laura
cz: (3) Jaroslav Jirků,Mr. Augustýn Jeřábek,Dr. Léda Husáková
de: (2) Martha Tidow,Hendrik Lack,Ms. Daria Müller
de_AT: (3) Mr. Ted Klimczak,Howard Einert,Lee Nytra
de_CH: (1) Loretta Bühlmann,Caleb Zimmermann,Alexis Schärer
dv: (1) ޖުވައިރިއްޔާ ދަލޫފް,އިބްރާހީމް ހައްފާފް,ޢާތިކާ ޣިނާ
el: (3) Dr. Bobbie Αναγνωστάκης,Mr. Bobby Μακρής,Dr. Eloise Πανταζής
en: (10) Casey Reynolds IV,Dr. Lester Parisian,Mrs. Marie Kassulke
en_AU: (0) Mr. Cedric Hudson Sr.,Sam Ernser,Dr. Shawna Hudson MD
en_AU_ocker: (0) Alfred Robinson II,Ralph Robinson,Ginger LeQuesne
en_BORK: (0) Jeannette Bosco,Jody Hilll,Ms. Carole Jakubowski
en_CA: (0) Lois Bergnaum,Darryl Rau,Catherine Mohr
en_GB: (0) Julio Rippin,Terrence Swift,Lauren Ledner
en_GH: (2) Baaba Wiafe,Christian Otiwa,Esi Baawia
en_IE: (0) Valerie Dooley Sr.,Dr. Rafael Rice,Miss Tracey Rippin
en_IN: (0) Jonathan Kaul,Jon Asan,Shari Kaur
en_NG: (2) Emmanuel Lawal,Akanni Onose,Aderonke Aminat
en_US: (0) Pedro Dibbert,Travis Corkery,Martha Yost
en_ZA: (2) Parnell Colleen,Dave Dube,Debbie Grant
es: (2) Ángel Posada Armendáriz,Jorge Pedraza Velázquez,Roser Guzmán Ocampo
es_MX: (4) Derrick Urrutia Manzanares,Moses Acosta de Tórrez,Carol Adorno Khalid
fa: (2) میلاد لاجوردی,Mr. مجید قنبری,آنیتا مفتاح
fi: (1) Lauri Turunen,Tapio Aaltonen,Kaarina Kallio
fr: (3) Poirier Claude,Leclercq Zacharie,Abigaïl Arnaud
fr_BE: (3) Bob Andre I,Zinedine Fournier IV,Benedicte Klocko Fils
fr_CA: (0) Taylor Schiller,Nelson Hammes,Gwendolyn Little
fr_CH: (3) Eleanor Küng,Mr. Alessio Bossy,Luena Geisendorf
ge: (2) Mr. Mack დავითიშვილი,Cedric ტაბაღუა,Bertha სულაშვილი
he: (2) Ms. נוי איש שלום,מיכאל אשל,אפריל חנני
hr: (3) Biserka Maruna,Dr. Roko Džakić,Adriana Raspudić
hu: (2) Dr. Balog Olívia,Kozma Dávid,Vincze Izabella
hy: (3) Նարինե Թադևոսյան PhD,Մոնթե Բաբայան Sr.,Եվա Գալստյան II
id_ID: (3) Titin Putri,Niyaga Mulyono Suwarno,Kiandra Rahimah
it: (2) Barsaba Campus,Mr. Elia Zamboni,Mrs. Melania Loffredo
ja: (1) 清水 彩花,渡辺 義雄,清水 貞子
ko: (1) 단 Margaret,황 Spencer,도 Ruby
lv: (6) Lība Auziņa k-dze,Legzdiņš Andris,Līga Amālija Zviedre
mk: (2) Галена Гермова,д-р Србо Кондовски,г-ѓа Грозда Василеска
nb_NO: (5) Anders Rasmussen III,Joakim Jacobsen Martinsen,Miss Oda Kristiansen
ne: (0) Ms. Lydia Niroula,Jake Adhikari,Nadine Basnet
nl: (4) Ms. Demi Brink,Nouri Koning I,Bowie Broek III
nl_BE: (3) Mrs. Stacy Vandamme,Theodore Declerck,Betsy Willems
pl: (2) Mrs. Klementyna Lipski,Dr. Atanazy Kwiecień,Mrs. Aida Dziuba
pt_BR: (3) Isis Melo,Lucas Xavier Jr.,Dr. Ana Luiza Souza
pt_PT: (2) Sra. Vânia Mota,Feliciano Albuquerque,Dra. Ana Tavares
ro: (3) Dr. Daniel Dascalu,Dr. Francisc Stoian,Safta Miu
ru: (4) Никон Артурович Борисов,Смирнов Аникей Исидорович,Алексеева Дарья
sk: (3) Klára Vicenová Phd.,Filip Kolár Phd.,Otília Pupáková
sv: (4) Lisa Abrahamsson Nordström,Kevin Nilsson Samuelsson,Camilla Ström Svensson
tr: (2) Dr. Peyami Başoğlu,Dr. Yahya Yetkiner,Mrs. Kamuran Orbay
uk: (4) Горова Добринка,Кулинич Сніжан Григорійович,Анна Арсеніївна Латанська
ur: (3) سیف شریف چہارم,Mr. احسن الحق,مشال اللَہ
vi: (1) Trường Liên Vương,Tường Anh Lâm,Hiếu Hạnh Tô
zh_CN: (1) 毛兰英,刘宇航,顾燕
zh_TW: (1) 賴Ramona,沈Jeffery,程Katie
zu_ZA: (0) Bhekizizwe Ndandali,Velaphi Zondi,Sindisiwe Moseley

src/modules/person/index.ts Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 7, 2022

Codecov Report

Merging #1637 (cea078e) into next (9c3618d) will decrease coverage by 0.01%.
The diff coverage is 99.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #1637      +/-   ##
==========================================
- Coverage   99.63%   99.63%   -0.01%     
==========================================
  Files        2346     2346              
  Lines      235002   235115     +113     
  Branches     1134     1132       -2     
==========================================
+ Hits       234147   234250     +103     
- Misses        833      843      +10     
  Partials       22       22              
Impacted Files Coverage Δ
src/definitions/person.ts 0.00% <0.00%> (ø)
src/locales/ar/person/name.ts 100.00% <100.00%> (ø)
src/locales/az/person/name.ts 100.00% <100.00%> (ø)
src/locales/cz/person/name.ts 100.00% <100.00%> (ø)
src/locales/de/person/name.ts 100.00% <100.00%> (ø)
src/locales/de_AT/person/name.ts 100.00% <100.00%> (ø)
src/locales/de_CH/person/name.ts 100.00% <100.00%> (ø)
src/locales/dv/person/name.ts 100.00% <100.00%> (ø)
src/locales/el/person/name.ts 100.00% <100.00%> (ø)
src/locales/en/person/name.ts 100.00% <100.00%> (ø)
... and 40 more

@matthewmayer
Copy link
Contributor Author

I have added a new helper function faker.helpers.weightedArrayElement as suggested.

I converted the existing name patterns to use weights (roughly, I tried to have prefixes/suffixes on 10-20% of the names only, people who know the locales better could tweak this later.)

@matthewmayer matthewmayer marked this pull request as ready for review December 7, 2022 11:51
@matthewmayer matthewmayer requested a review from a team as a code owner December 7, 2022 11:51
@ST-DDT
Copy link
Member

ST-DDT commented Dec 9, 2022

I have added a new helper function faker.helpers.weightedArrayElement as suggested.

Could you please move that to a separate PR? This is getting harder and harder to review.

@ST-DDT ST-DDT added c: bug Something isn't working p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug m: person Something is referring to the person module labels Dec 9, 2022
@ST-DDT ST-DDT added this to the v8.0 - Module Re-Shuffling milestone Dec 9, 2022
@matthewmayer
Copy link
Contributor Author

I will change this back to draft for now and split into some smaller PRs

@matthewmayer matthewmayer marked this pull request as draft December 11, 2022 02:44
@matthewmayer
Copy link
Contributor Author

Note i will redo a simplified version of this PR once #1654 and #1665 have been merged.

One thing i was thinking, person/name.ts is not the most descriptive name. Maybe this could be modified to person/name_patterns.ts or person/full_name_patterns.ts as part of this PR?

@ST-DDT
Copy link
Member

ST-DDT commented Dec 18, 2022

One thing i was thinking, person/name.ts is not the most descriptive name. Maybe this could be modified to person/name_patterns.ts or person/full_name_patterns.ts as part of this PR?

You are right. We also considered renaming other pattern files such as location street and city pattern files.

I'm not sure whether it should be done in a separate PR though (to split simple renames from functional changes and locale additions).

@matthewmayer
Copy link
Contributor Author

sure, it can wait until a later PR.

ST-DDT
ST-DDT previously approved these changes Jan 29, 2023
@ST-DDT ST-DDT requested review from a team January 29, 2023 20:54
@ST-DDT ST-DDT enabled auto-merge (squash) February 1, 2023 15:41
@ST-DDT
Copy link
Member

ST-DDT commented Feb 1, 2023

Could you please fix the new lint error?

auto-merge was automatically disabled February 2, 2023 02:07

Head branch was pushed to by a user without write access

src/definitions/person.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT merged commit 1ae2f6f into faker-js:next Feb 2, 2023
@matthewmayer
Copy link
Contributor Author

Yay! Thanks for all the help with this

@matthewmayer matthewmayer mentioned this pull request Feb 8, 2023
10 tasks
matthewmayer pushed a commit to matthewmayer/faker that referenced this pull request Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working m: person Something is referring to the person module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add the concept of order for firstName and lastName in name.fullName
4 participants