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

Add support for coordinates resolution. #209

Merged
merged 2 commits into from
Aug 11, 2023

Conversation

keyurva
Copy link
Contributor

@keyurva keyurva commented Aug 10, 2023

  • For Support name and lat/lng based resolution in dc-import tool #203
  • Coordinates resolution is only enabled if a new --coordinates-resolution flag is set.
  • The implementation is intentionally minimally invasive to the existing resolution code path and is executed only when the above flag is set.
  • Currently it only supports numeric latitude and longitude properties and basic validation. Support for more formats and validations will be added subsequently.

Copy link
Contributor

@pradh pradh left a comment

Choose a reason for hiding this comment

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

Nice!

Please consider updating docs in a follow up.

https://github.com/datacommonsorg/import/tree/master/docs

util/src/main/java/org/datacommons/util/ReconClient.java Outdated Show resolved Hide resolved
chunkedResponse ->
chunkedResponse.getPlaceCoordinatesList().stream())
.collect(toList()))
.build());
Copy link
Contributor

Choose a reason for hiding this comment

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

please add some comments for this block?

import org.datacommons.proto.Recon.ResolveCoordinatesResponse;
import org.datacommons.proto.Recon.ResolveCoordinatesResponse.Place;

/** Resolves nodes with lat-lngs by calling the DC coordinates resolution API. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Add TODO to update counters on errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@keyurva
Copy link
Contributor Author

keyurva commented Aug 11, 2023

Nice!

Please consider updating docs in a follow up.

https://github.com/datacommonsorg/import/tree/master/docs

Thanks Prashanth. Will do.

@keyurva keyurva merged commit cbfe2c1 into datacommonsorg:master Aug 11, 2023
@keyurva keyurva deleted the resolve-coordinates branch November 16, 2023 19:12
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.

2 participants