2669 improve location accuracy for street newsflashes #2708
Conversation
…ove-location-accuracy-for-street-newsflashes
…ove-location-accuracy-for-street-newsflashes
…ove-location-accuracy-for-street-newsflashes
…ove-location-accuracy-for-street-newsflashes # Conflicts: # main.py
|
@tkalir no urgency here, whenever you have time - can you please check and fix Pylint, Black and failing tests? |
…ove-location-accuracy-for-street-newsflashes
| openai==1.45.0 | ||
| langchain==0.2.16 | ||
| langchain_openai==0.1.25 | ||
| python-dotenv |
There was a problem hiding this comment.
Is python-dotenv necessary?
Also, SQLAlchemy==1.4 modification necessary?
(I think we should perform packages upgrade, but it will be in a different pr with suitable tests)
| @@ -282,11 +307,12 @@ def reverse_geocode_extract(latitude, longitude): | |||
| try: | |||
| gmaps = googlemaps.Client(key=secrets.get("GOOGLE_MAPS_KEY")) | |||
| geocode_result = gmaps.reverse_geocode((latitude, longitude)) | |||
|
|
|||
| print(geocode_result) | |||
There was a problem hiding this comment.
Can you remove all prints in this code?
There was a problem hiding this comment.
@tkalir All in all looks good., added some small comments
In addition, in the case that location is on a "boarder" of two municipalities and nearest accidents might be in different "yishuv_name", this case is not handled.
I think this should be taken into account in the current solution which might modify it a bit.
|
@ziv17 can you review this one as well? (No urgency here) |
|
@tkalir I added key to secrets. |
| from enum import Enum | ||
| from anyway import secrets | ||
|
|
||
| api_key = secrets.get("OPENAI_API_KEY") |
There was a problem hiding this comment.
I suggest inserting the secret into a function, to avoid tests failures in tests from forks
| model = ChatOpenAI(api_key=api_key, temperature=0) | ||
|
|
||
|
|
||
| def match_streets_with_langchain(street_names, location): |
There was a problem hiding this comment.
Hi, I prefer to have type hints. It makes the reading much easier.
| model = ChatOpenAI(api_key=api_key, temperature=0) | ||
|
|
||
|
|
||
| def match_streets_with_langchain(street_names, location): |
There was a problem hiding this comment.
Hi @tkalir , Very nice!
I learned a lot 😊
- I assume this PR is only the beginning, and probably it is possible to extract more information, like the yishuv, resolution, etc.
- Is it possible to get the level of confidence the AI tool is has in its answer? In certain cases it may be more accurate than the lat / lon that we have.
No description provided.