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

Phonenumber field difference for Python 2 and 3 #136

Closed
neequole opened this issue Aug 3, 2016 · 8 comments
Closed

Phonenumber field difference for Python 2 and 3 #136

neequole opened this issue Aug 3, 2016 · 8 comments

Comments

@neequole
Copy link

neequole commented Aug 3, 2016

Hi Guys,

Just a quick question.. why is that for Python 2, if the phone number is invalid it just returns the raw input while in Python 3, it converts it to either +NoneNone or +63(region code, mine is PH)(Raw Input) ?

Below are what i have tried.
Python2
print to_python('none') - none
print to_python('None') - None
print to_python('9581093') - 9581093
print PhoneNumber.from_string(phone_number='123') - 123

Python3
print(to_python('none')) - +NoneNone
print(to_python('None')) - +NoneNone
print(to_python('9581093')) - +639581093
print(PhoneNumber.from_string(phone_number='213')) - +63213

@Shazam14
Copy link

Yes I have the same issue, it seems this was not resolved.

@amateja
Copy link
Collaborator

amateja commented Jun 26, 2018

Please consider following script:

import phonenumbers
from django.conf import settings

from phonenumber_field.phonenumber import PhoneNumber, to_python

settings.configure(PHONENUMBER_DEFAULT_REGION='PH')

assert str(to_python('none')) == '+NoneNone'
assert str(to_python('None')) == '+NoneNone'
assert str(to_python('9581093')) == '+639581093'
assert str(PhoneNumber.from_string(phone_number='123')) == '+63123'

I've run it successfully on both python 2.7.12 and python 3.6.5 with django-phonenumber-field 2.0.0 installed. So I couldn't reproduce your issue.

@stefanfoulis
Copy link
Owner

@neequole and @amateja with which version of django-phonenumber-field have you each tested?

And besides the possible inconsistency between python versions: What do we want as a reasonable output for those inputs?

@amateja
Copy link
Collaborator

amateja commented Jul 1, 2018

In my very first post I wrote exact version of python and django-phonenumber-field used 😉. To be even more specific:

Successful cases

python2

  • python 2.7.12
  • Django 2.0.6
  • django-phonenumber-field 2.0.0
  • phonenumberslite 8.9.9

python3

  • python 3.6.5
  • Django 2.0.6
  • django-phonenumber-field 2.0.0
  • phonenumberslite 8.9.9

Unsuccessful cases

python2

  • python 2.7.12
  • Django 1.11.13
  • django-phonenumber-field 1.0.0
  • phonenumbers 8.9.9

python3

  • python 3.6.5
  • Django 2.0.6
  • django-phonenumber-field 1.0.0
  • phonenumbers 8.9.9

My bottom line is it not related with version of python or Django or phonenumbers*. It was a change between django-phonenumber-field version 1.0.0 and 1.1.0. Therefore there is noting to fix.

I think there is no reasonable output for those inputs. This library is very lenient... perhaps too much. I would rise ValueError on the very beginning not allowing such inputs at all.

@Shazam14
Copy link

HI @amateja

Where did you modify this is it inside your library or I need to create a new class.

import phonenumbers
from django.conf import settings

from phonenumber_field.phonenumber import PhoneNumber, to_python

settings.configure(PHONENUMBER_DEFAULT_REGION='PH')

assert str(to_python('none')) == '+NoneNone'
assert str(to_python('None')) == '+NoneNone'
assert str(to_python('9581093')) == '+639581093'
assert str(PhoneNumber.from_string(phone_number='123')) == '+63123'

@amateja
Copy link
Collaborator

amateja commented Jul 14, 2018

This is a standalone script not a copy-paste solution for your problem. If you use django-phonenumber-field in version 1.0.0 you get AssertionError regardless python version run. Using anything more up to date e.g. recent version 2.0.0 solves your problem.

To test my theses please just launch your python interpreter and copy-paste this sample code. If you find I'm wrong please attach output of this script and add result of pip freeze command.

@Shazam14
Copy link

HI, I had this figured out,
got confused where to place that, but again through reading /testing was able to solved it.

Thank you for this!

@amateja
Copy link
Collaborator

amateja commented Jul 16, 2018

Glad I could help. Cheers!

@amateja amateja closed this as completed Jul 16, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants