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

Added example to demonstrate use georadius. #104

Merged
merged 5 commits into from
Apr 22, 2021

Conversation

mr-shitij
Copy link
Contributor

According to standards and as used in redis server we always specify latitude first and then longitude. That's why geofilter is not working in redisearch-go client. I Fixed that bug by changing their position and also added an example to demonstrate how to use geofilter filter in redisearch-go client.

I hope you will consider.

@codecov
Copy link

codecov bot commented Apr 16, 2021

Codecov Report

Merging #104 (61c97c4) into master (75f19ff) will increase coverage by 0.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
+ Coverage   76.63%   76.81%   +0.18%     
==========================================
  Files          13       13              
  Lines        1100     1100              
==========================================
+ Hits          843      845       +2     
+ Misses        199      198       -1     
+ Partials       58       57       -1     
Impacted Files Coverage Δ
redisearch/client.go 79.78% <0.00%> (+0.54%) ⬆️

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 75f19ff...61c97c4. Read the comment docs.

@filipecosta90
Copy link
Collaborator

@mr-shitij GEOFILTER is expressed as long,lat as clearly stated on docs:

GEOFILTER {geo_field} {lon} {lat} {radius} m|km|mi|ft : If set, we filter the results to a given radius from lon and lat. Radius is given as a number and units. See GEORADIUS for more details.

Same applies to the redis GEORADIUS command:

GEORADIUS key longitude latitude radius m|km|ft|mi [WITHCOORD] [WITHDIST] [WITHHASH] [COUNT count [ANY]] [ASC|DESC] [STORE key] [STOREDIST key]

Noneteless I'll fix your example, add another godoc example to make it clear.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@filipecosta90 filipecosta90 changed the title Bug Fixed In GEOFILTER Serialisation and added example to demonstrate use geofilter. Added example to demonstrate use georadius. Apr 22, 2021
@filipecosta90
Copy link
Collaborator

@mr-shitij notice I've updated your PR given there was no real bug on the client. I made usage of your example and fixed it. Also included the godoc variant so that people seeing our docs can check It.
Thank you for the PR. Merging!

@filipecosta90 filipecosta90 merged commit 8df3865 into RediSearch:master Apr 22, 2021
@mr-shitij
Copy link
Contributor Author

Thank You ..!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants