-
Notifications
You must be signed in to change notification settings - Fork 132
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
Add utility method to format phone numbers #928
Add utility method to format phone numbers #928
Conversation
IMO a better default behavior for a format phone number method would be to return None for an invalid phone number rather than raise an error. |
That makes sense to me. I can update that. |
Another option would be to add some kind of |
I'm happy to do either one. Do folks think my approach to not adding another dependency makes sense? I'm also willing to write a function that uses the phonenumbers package. |
Yeah I think your approach is good! The dependency seems unnecessary. A few other validations worth incorporating potentially - valid US phones can't have an area code that starts with 0 or 1, and the first digit after the area code also can't be 0 or 1. I think 999 is also an invalid area code. FWIW the phonenumbers package doesn't invalidate those patterns so there's no advantage there either. |
It might be worth looking at the things phonenumbers does validate in case there's anything we want to copy. Although I took a quick look at the docs/code and it's not clear to me what their full list of checks are. Oh well! Regardless, I agree that it's overkill given Parsons users are primarily/mostly using US numbers. |
|
||
def format_phone_number(phone_number, country_code="1"): | ||
""" | ||
Formats a phone number in E.164 format. |
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.
Could you add some documentation showing what it'll do? That would be helpful to someone who doesn't know what the e.164 format is, and might have no idea what this does.
Even one line like:
Example: Converts "(123) 123-1212" to "+01231231212"
would be very helpful!
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.
Sounds good. Went ahead and made that change.
This pull request adds a new utility method,
format_phone_number
, which formats a phone number in E.164 format. The method takes a phone number as input and an optional country code, and returns the formatted phone number. It removes non-numeric characters and leading zeros, adds the country code prefix if necessary, and formats the number in E.164 format. The pull request also includes unit tests for theformat_phone_number
method to ensure its correctness and handle different scenarios.