Mae/business name#90
Conversation
mae-smarty
requested review from
andrea-wait-smarty,
andyjohnsonsmarty and
timothy-smarty
There was a problem hiding this comment.
Need to update the Makefile to include this file
|
|
||
| class Lookup(LookupBase): | ||
| def __init__(self, smartykey=None, dataset=None, dataSubset=None, features=None, freeform=None, street=None, city=None, state=None, zipcode=None): | ||
| def __init__(self, smartykey=None, dataset=None, dataSubset=None, features=None, freeform=None, business_name=None, street=None, city=None, state=None, zipcode=None): |
There was a problem hiding this comment.
I'm not 100% sure how customers are using the Python SDK, but we should probably move string businessName = null to the end of the constructor, in the case that someone was doing this previously:
Lookup(null, null, null, null, "123 main st", "123 main st");
In this case, 123 main st was originally the street parameter, but now it is the businessName parameter. It's an edge case, but I would do this instead:
def __init__(self, smartykey=None, dataset=None, dataSubset=None, features=None, freeform=None, street=None, city=None, state=None, zipcode=None, business_name=None):
| self.dataSubset = dataSubset | ||
| self.features = features | ||
| self.freeform = freeform | ||
| self.business_name = business_name |
There was a problem hiding this comment.
You don't have to change this because I don't think there's a right answer in this repo, but in this file, two word variables are either camel case case (dataSubset) or lowercase (smartykey). In other lookup files, they do follow snake case. So anyways I just had to point that out.
| and _is_blank(getattr(lookup, 'business_name', None)) | ||
| ): | ||
| raise SmartyException("Lookup requires one of 'smartykey', 'street', or 'freeform' to be set") | ||
| raise SmartyException("Lookup requires one of 'smartykey', 'street', 'freeform', or 'business_name' to be set") |
There was a problem hiding this comment.
If you make a change according to my Lookup comment, I would change this too to either be smarty_key or businessName
2 participants
Added the business_name input parameter and associated testing and examples.