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

Update geocoder_api.py #77

Merged
merged 3 commits into from
May 14, 2024
Merged

Conversation

antosomzi
Copy link
Contributor

Hello,

I have upgrade the geocoding functionality in the GeocoderApi class to accommodate scenarios where only partial address information, such as city and postcode, is available. Currently, the address_with_details method raises errors when incomplete address details are provided.

Upgrade the geocoding functionality in the GeocoderApi class to accommodate scenarios where only partial address information, such as city and postcode, is available. Currently, the address_with_details method raises errors when incomplete address details are provided. As most of the companies are from scrapped data or hubspot and full address is not available, we can't create new companies in the DirectoryCompany table.
house_number: int,
street: str,
house_number: Optional[int] = None,
street: Optional[str] = None,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the optional parameters after the required parameters to fix the linting issues

fix linting issues
city: str,
country: str,
lang: str = "en-US",
house_number: Optional[int] = None,
street: Optional[str] = None,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

@@ -129,11 +129,16 @@ def address_with_details(
Raises:
HEREError"""

qq_query = ""
if house_number is not None:
qq_query += f"houseNumber={house_number};"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add new test cases to cover the optional parameters?

if house_number is not None:
qq_query += f"houseNumber={house_number};"
if street is not None:
qq_query += f"street={street};"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@codecov-commenter
Copy link

codecov-commenter commented May 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.24%. Comparing base (16b9261) to head (fb45fa5).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #77      +/-   ##
==========================================
+ Coverage   93.22%   93.24%   +0.02%     
==========================================
  Files          26       26              
  Lines        1726     1733       +7     
==========================================
+ Hits         1609     1616       +7     
  Misses        117      117              
Flag Coverage Δ
unittests 93.24% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@abdullahselek
Copy link
Owner

Thanks for adding new test cases, looks good!

@abdullahselek abdullahselek merged commit 8fc6b96 into abdullahselek:master May 14, 2024
22 checks passed
@antosomzi
Copy link
Contributor Author

Thanks for adding new test cases, looks good!

thank you for the quick response !

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

Successfully merging this pull request may close these issues.

3 participants