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

feat: Add support for Azure Maps Search APIs #1294

Merged
merged 10 commits into from
Dec 10, 2021
Merged

feat: Add support for Azure Maps Search APIs #1294

merged 10 commits into from
Dec 10, 2021

Conversation

SudhindraKovalam
Copy link
Contributor

  • Add support for Azure Maps Get Search API
  • Add support for Batch Search API

- Created a geospatial package to support all geospatial functions
- Added support for GetSearchAddress
- TODO: Fix the flattening of batch results
- fixed how the columns are fetched and flattened
- added asserts to ensure the results `df` contains all rows
@SudhindraKovalam
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2021

Codecov Report

Merging #1294 (5893352) into master (7c278b7) will decrease coverage by 0.31%.
The diff coverage is 10.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1294      +/-   ##
==========================================
- Coverage   78.04%   77.73%   -0.32%     
==========================================
  Files         271      273       +2     
  Lines       13397    13453      +56     
  Branches      681      689       +8     
==========================================
+ Hits        10456    10458       +2     
- Misses       2941     2995      +54     
Impacted Files Coverage Δ
...azure/synapse/ml/geospatial/AzureMapsHelpers.scala 4.25% <4.25%> (ø)
.../azure/synapse/ml/geospatial/AzureMapsSearch.scala 44.44% <44.44%> (ø)
...crosoft/azure/synapse/ml/io/http/HTTPClients.scala 51.66% <0.00%> (-3.34%) ⬇️
...ure/synapse/ml/io/http/SimpleHTTPTransformer.scala 85.93% <0.00%> (-3.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c278b7...5893352. Read the comment docs.

@SudhindraKovalam
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@SudhindraKovalam SudhindraKovalam changed the title feat) Add support for Azure Maps Search APIs feat: Add support for Azure Maps Search APIs Dec 8, 2021
@SudhindraKovalam SudhindraKovalam changed the title feat: Add support for Azure Maps Search APIs Add support for Azure Maps Search APIs Dec 8, 2021
@SudhindraKovalam SudhindraKovalam changed the title Add support for Azure Maps Search APIs feat: Add support for Azure Maps Search APIs Dec 8, 2021
@SudhindraKovalam
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@mhamilton723 mhamilton723 left a comment

Choose a reason for hiding this comment

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

Looks really good! A few questions for you

- [Review Comment] Removed `AzureMapsBase` class to simplify dependency
- [Review Comment] Renamed `BatchAsyncReply` to `MapsAsyncReply`
- [Review Comment] Removed `=None` assignments in schema classes
- [Enhancements] Update Output Column to only return batch results making is easier to flatten responses.
- Updated test case to include realistic address data
- fixed a potential issue with `MapAsyncReply` appending subscription key twice
- Explicitly set the user agent header to track SynapseML usage
- cleaned up `HasAddressInput`
@mhamilton723
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mhamilton723
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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.

None yet

3 participants