search

move-coop / parsons

A python library of connectors for the progressive community.
https://www.parsonsproject.org/
Other
260 stars 132 forks source link

Add utility method to format phone numbers #928

Closed mkrausse-ggtx closed 9 months ago

mkrausse-ggtx commented 10 months ago

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 the format_phone_number method to ensure its correctness and handle different scenarios.

austinweisgrau commented 10 months ago

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.

mkrausse-ggtx commented 10 months ago

That makes sense to me. I can update that.

shaunagm commented 10 months ago

Another option would be to add some kind of error_on_invalid option which defaults to false, but allows people to proactively say "yes this absolutely needs a number, error if it doesn't exist". Users could always write code to check the output and error on none, but it might be worth making it more convenient for them.

mkrausse-ggtx commented 10 months ago

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.

austinweisgrau commented 10 months ago

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.

shaunagm commented 10 months ago

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.