-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add firstname gender method to all Person provider #114
Add firstname gender method to all Person provider #114
Conversation
I've made some changes to the nl_NL provider. While it doesn't yet reflect your changes, it does add separation to Famale/Male first names. #135 |
Thanks dynom, i added your firstnames list into nl_NL Person. |
When the PR is accepted, I'll take care of pt_BR. |
Awesome @csanquer, thanks! |
Any news on this PR ? |
I' m working on it again, and i'll update this PR with recent Master changes |
I just made a PR #276 for pl_PL yesterday. |
); | ||
|
||
// TODO separate male and female title | ||
protected static $titleMale = array('mgr','inż.', 'dr', 'doc.'); |
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.
Those are academic degrees and they're unisex. Mr and Ms equivalents are:
- Pan for male
- Pani for female
I don't think they should be included in default patterns, why some names should have the Mr/Ms prefix and others don't? For academic degrees you either have some or any at all.
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.
Thanks for your explanation, I will update that.
I ran php-cs-fixer and it seems this PR needs rebase... |
ok I forgot to use php-cs-fixer but the PR is not yet ready, there are still 14 remaining locale provider to update and split firstnames |
Maybe this could be split into more PRs? Seems like an awful lot of work and it already waits for quite long time. |
I rebased and updated more i18n providers but I still need help for the remaining locales. I used REST API like genderize.io or gender-api.com but even with these usefull web services, some firstname genders remain unknown. |
I updated all person providers and splitted all firstnames, titles and name formats into male and female. Some providers have commented unknown gender firstnames (en_ZA, lv_LV, sr_*, zh_CN). I rebased and squashed the PR. It's ready for merge. |
titleMale // 'Mr.' | ||
titleFemale // 'Ms.' | ||
suffix // 'Jr.' | ||
name($gender = null) // 'Dr. Zane Stroman' // false, 0, 'male', 'm', 'boy' ,'b', 'man' for male; true, 1, 'female', 'f', 'girl', 'g', 'w', 'woman' for female; null or other value for any male or female firstname |
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.
You should "hide" all the possible choices in the $gender
value - they make the doc more complex to read. It's ok to leave all those details in the PHPDoc, but here in the README, you should only show $gender = 'male|female'
Please revert work on locales where you have undefined gender - this reduces the randomness of data. |
I followed your advices and improved the implementation and restore undefined gender firstnames into male and female firstnames list |
Thanks @csanquer hopefully this gets merged soon. |
} | ||
|
||
/** | ||
* @example 'Doe' | ||
*/ | ||
public static function lastName() | ||
public function lastName() |
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.
why not static anymore?
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.
Because in some I18N Person providers like bg_BG , I need to use $this->generator->parse otherwise some tests failed
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.
and I follow your advice to refactore using more formats
Is there anything that prevents merging this PR ? |
👍 There's a lot of people waiting for this PR to get merged. Lot of work in it. |
Yes I know, maybe @fzaninotto is very busy these days. He was at the symfony live in Paris this week. |
Needs rebase |
); | ||
|
||
/** | ||
* @link http://sr.wikipedia.org/wiki/%D0%A1%D0%BF%D0%B8%D1%81%D0%B0%D0%BA_%D1%81%D1%80%D0%BF%D1%81%D0%BA%D0%B8%D1%85_%D0%B8%D0%BC%D0%B5%D0%BD%D0%B0 |
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.
Why did you remove the source? It's necessary.
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.
Oops !! Maybe, a rebase conflict not correctly resolved.
add FirstNameMale and FirstNameFemale method to all Person classes add gender parameter to some localized lastname and prefix method update README add missing prefix method for bg_BG Person provider set male and female firstnames for netherland locales refactore is_IS firstname providers add more values for gender parameter rename person prefix methods to title and refactore them separate male and female en_US firstname update all Person Providers add some unit tests for man, woman firstname and title replace male and female title by academic degrees only for pl_PL add polish names source comment update male and female firstname for fr_FR, fr_CA and fr_BE update male and female firstnames for es_AR, es_ES and es_PE update male and female firstnames for pt_BR update male and female firstnames and titles for it_IT change title behavior for cs_CZ update male and female firstnames for da_DK change title behavior for nl_NL update male and female firstnames for hu_HU fix PSR2 coding style fix PSR2 coding style update male and female bengali titles fix firstnames unit tests for me_ME update male and female firstnames and titles for en_ZA and fi_FI (some names stay with gender unkown) update male and female firstnames for lv_LV update male and female firstnames for tr_TR update male and female firstnames for zh_CN update male and female firstnames for ja_JP update male and female firstnames for fi_FI update male and female firstnames for bn_BD update male and female firstnames for ru_RU fix title method for sk_SK update partially male and female firstnames for sr_* update README remove deprecated comments update male and female firstnames and title for hy_AM update male and female firstnames for uk_UA add gender option to name formatter fix PSR2 coding style make gender option more simple and improve implementation restore unknown gender firstnames into male and female firstnames list update es_ES firstnames list restore source link for sr_RS Person firstname provider simplify Person name unit test
i have fixed the lines you comment and rebase and squash the PR |
Let's get this rolling |
Add firstname gender method to all Person provider
Thanks a lot for all your work on this PR, @csanquer! This is much appreciated. |
I hope just someone can update the last providers which I can't split firstnames. |
Hi,
This is a missing feature already implemented in some localized Person provider but useful to all Person Providers.
The refactoring is not completed and I need some help to separate male and female firstname.
Concerned files have this comment :
// TODO separate male and female firstname
Remaining localized Person classes which need male and female firstname separating :
bn_BDbg_BGcs_CZda_DKde_ATde_DEel_GRen_AUen_CAen_GBen_PHen_USes_ARes_ESes_PEfi_FIfr_BEfr_CAfr_FRhu_HUhy_AMis_ISit_ITja_JPnl_BEnl_NLme_MEpl_PLpt_BRro_MDro_ROru_RUsk_SKtr_TRuk_UASome firstnames remain unknown