mocnik-science / osm-python-tools

A library to access OpenStreetMap related services
GNU General Public License v3.0
440 stars 48 forks source link

TypeError: __init__() got an unexpected keyword argument 'cacheDir' #48

Closed WolfgangFahl closed 2 years ago

WolfgangFahl commented 2 years ago
 nominatim = Nominatim(cacheDir=".nominatim")

used to work prior to version 0.3.3 - now it's giving an error message althought the description still describes this parameter. https://github.com/mocnik-science/osm-python-tools/commit/76c5f30ac64789e5a927837f9b5a0faa20e06598 seems to be the issue. You might want to add Continuuous Integration tests to avoid such issues since the problem showed up in our own CI tests and debugging the situation lead to this ticket.

mocnik-science commented 2 years ago

Thank you for pointing out. The documentation was updated after releasing v0.3.3.

You find the documentation of the version including the breaking changes in the file version-history.md.

I can confirm that the following code does not work any longer:

from OSMPythonTools.overpass import Overpass
overpass = Overpass(cacheDir='.nominatim')

Instead, I have introduced a new CachingStrategy, which allows for even more flexibility.

from OSMPythonTools.cachingStrategy import CachingStrategy, JSON
from OSMPythonTools.overpass import Overpass

CachingStrategy.use(JSON, cacheDir='.nominatim')
overpass = Overpass()

You can find the documentation of the CachingStrategy in the General Remarks Section.

I hope this explains well and helps.

WolfgangFahl commented 2 years ago

where did Nominatim go which i am using? The docs are at https://github.com/mocnik-science/osm-python-tools/blob/master/docs/nominatim.md and still have

nominatim = Nominatim(cacheDir='cache')

Looks that that constructor needs to call the new caching strategy internally to hide the details. Is that a different project so that and issue needs to be created there? Doesn't there have to be a CI test case checking the above line of code as is being the case in my own project? For the time being i have to stick to my workaround of pinning the old version since the comments here did not clarify the issue i was hoping to point out.

mocnik-science commented 2 years ago

Thanks again for pointing out. The sentence has now been removed from the documentation because it is not longer applicable. You can use Nominatim in a very similar way than before. Only the caching strategy needs to be configured separately in order to allow for more flexibility:

from OSMPythonTools.cachingStrategy import CachingStrategy, JSON
from OSMPythonTools.nominatim import Nominatim

CachingStrategy.use(JSON, cacheDir='.nominatim')
nominatim = Nominatim()

Doesn't there have to be a CI test case checking the above line of code as is being the case in my own project? There are tests in place which I run every time before releasing a new version. As this is an intended change, a CI would not be of any help here. I know that I changed the way these parameters have to be provided and documented this carefully (well, I forgot to adapt the one sentence of the documentation). A CI would only help me to identify that something does not work as expected. I am, however, well aware of that breaking change.