Description:
Seems like @duckyou have found a bug in our code and seed is really does not work sometime and we should fix it.
Reason:
It's happening because we use custom_code which use another random object.
Decision:
We should move utils which use random to helpers.Random
Thanks for the quick response, @lk-geimfari :smile:
I can try moving custom_code to BaseProvider like you said in #324 discussion
@duckyou You know, it's better to move it to helpers.Random. There's just everything related to random selection.
@duckyou after that, we can just call self.random.custom_code. Looks good.
@duckyou If you don't want to do it, I can do it by myself, but if you want work on it, then, please, create new PR, because old is really hard to read and review :smile: . Thanks!
@lk-geimfari Hmm... You goddamn right!
That's may be simplest solution :relaxed:
I do this ;)
@duckyou Thank you!
@lk-geimfari
https://github.com/lk-geimfari/mimesis/blob/af9c877d9bde654dff2f8be524b81a35460f2307/mimesis/enums.py#L6-L12
Method get_random_item also affected :worried:
@duckyou We will not change it, because it's not really about data, but about arguments for function.
@lk-geimfari Issue explain:
>>> from mimesis import Address
>>> from mimesis.enums import CountryCode
>>> a = Address(seed=42)
>>> a.country_iso_code(fmt=CountryCode.ISO3)
'NIC'
>>> a.country_iso_code(fmt=CountryCode.ISO3)
'BRN'
>>> a.country_iso_code(fmt=CountryCode.ISO3)
'ARM'
>>> a = Address(seed=42)
>>> a.country_iso_code(fmt=CountryCode.ISO3)
'NIC'
>>> a.country_iso_code(fmt=CountryCode.ISO3)
'BRN'
>>> a.country_iso_code(fmt=CountryCode.ISO3)
'ARM'
...
This worked because we set format directly
... but if we omit this parameter
...
>>> a = Address(seed=42)
>>> a.country_iso_code()
'558'
>>> a.country_iso_code()
'096'
>>> a.country_iso_code()
'ARM'
>>> a = Address(seed=42)
>>> a.country_iso_code()
'NIC'
>>> a.country_iso_code()
'096'
>>> a.country_iso_code()
'051'
its happen beause:
https://github.com/lk-geimfari/mimesis/blob/af9c877d9bde654dff2f8be524b81a35460f2307/mimesis/providers/address.py#L147-L148
@duckyou Skip this for now. I'll think about it later. Our priority for a right now is custom_code.
Fixed. Now we should think about how should we test providers with seed.
@lk-geimfari good job! 馃樃
Sorry for off-top, but maybe this project community have a place for discussions.. for example irc or slack?
@duckyou It's okay. Unfortunately, not yet. We have a small community, so usually, we discuss everything here, although it would be great, of course, to have a place where we could discuss everything.
We can create repository discussion in @mimesis-lab. I can add you to the team if you're interested in this project.
@duckyou I have invited you to @mimesis-lab. Accept the invitation if you're interested. Thanks!
@lk-geimfari im in! ^^
@lk-geimfari don't forget about https://github.com/lk-geimfari/mimesis/issues/325#issuecomment-352364359 ?
Heres two problems: AllowRandom.get_random_item and ValidateEnumMixin._validate_enum
Ok.. firstly how to get random enum with seed?
Original realisation is:
https://github.com/lk-geimfari/mimesis/blob/217e5fa7775389b92a166f643276b2533a5fb45e/mimesis/enums.py#L11-L12
aka we can just do this self.random.choice(list(CountryCode)) without mentioned method
and if we create this simple hack in AllowRandom metaclass:
def __getitem__(self, index):
if isinstance(index, int):
return list(self._member_map_)[index]
return super().__getitem__(index)
we can do now self.random.choice(CountryCode) :wink:
@duckyou Also we can add the method get_random_item() to helpers.Random.
def get_item(self, enum: Optional[Any] = None):
return self.choice(list(self))
What do you think? In my opinion, it will be easier.
In this case, we should move _validate_enum() to BaseProvider
@lk-geimfari
hmmm self.random.get_enum_item(CountryCode) or self.random.choice(CountryCode)
... also enum attribute always should be passed to get_enum_item method
but if we combine this with _validate_enum...
key = self.random.get_enum_item(fmt, CountryCode)
Something like this:
def get_enum_item(self, enum: Any, item: Any = None) -> Any:
if item:
raise NonEnumerableError(enum) if item not in enum
return item
return self.choice(list(enum))
@duckyou Stop stop. Seems like the solution which I suggested above will not work, it seems.
Do you suggest something like this?
class Enum(enum.Enum):
def __getitem__(self, index):
if isinstance(index, int):
return list(self._member_map_)[index]
return super().__getitem__(index)
class PortRange(Enum):
ALL = (1, 65535)
WELL_KNOWN = (1, 1023)
EPHEMERAL = (49152, 65535)
REGISTERED = (1024, 49151)
@duckyou The problem that we need to handle different cases. For example, sometimes we need the value of the enum, but sometimes we need item of enum object, i.e Enum.ITEM, not Enum.ITEM.value.
Oh... here is:
class MimEnumMeta(EnumMeta):
def __getitem__(self, index):
if isinstance(index, int):
return list(self)[index]
return super().__getitem__(index)
class PortRange(Enum, metaclass=MimEnumMeta):
ALL = (1, 65535)
WELL_KNOWN = (1, 1023)
EPHEMERAL = (49152, 65535)
REGISTERED = (1024, 49151)
...
Now:
>>> from mimesis import enums
>>> import random
>>> pr = enums.PortRange
>>> pr
<enum 'PortRange'>
>>> pr['ALL']
<PortRange.ALL: (1, 65535)>
>>> pr.ALL
<PortRange.ALL: (1, 65535)>
>>> pr[0]
<PortRange.ALL: (1, 65535)>
>>> random.choice(pr)
<PortRange.REGISTERED: (1024, 49151)>
>>> random.choice(pr) in enums.PortRange
True
>>> random.choice(pr).value
(1, 1023)
>>> random.choice(pr).name
'EPHEMERAL'
@duckyou I'll wait for your PR. Keep in mind that we have builtins data providers and that sometimes we need an item of the enum, but not a value (personla.py -> surname, payment.py -> credit_card_number).
@lk-geimfari Can i also move _validate_enum() to BaseProvider, like you say here https://github.com/lk-geimfari/mimesis/issues/325#issuecomment-353350713 ?
@duckyou If we do it, then tests will fail because ValidateEnumMixin used in mimesis.builtins. It's really hard to understand what is better. At this moment we can only experiment with solutions.
@lk-geimfari I see here several solutions:
@duckyou Let's will weigh the pros and cons of all solutions.
@duckyou We need to take a decision that will allow us to make the minimum number of changes.
@duckyou I have found a new problem:
Seeded full_name() returns different data because there are two different methods with similar seed, i.e name and surname.
And this is only the tip of the iceberg I think. Now I'm tired and can not figure out why the data is changing for fixed seed, sorry.
I have created PR where you can see changes: https://github.com/lk-geimfari/mimesis/pull/332
Seems like builtins providers also does not support of seed. We should fix it!
Most helpful comment
@lk-geimfari im in! ^^