openplans / openblock

OpenBlock is a web application and RESTful service that allows users to browse and search their local area for "hyper-local news
61 stars 26 forks source link

convert_to_block=True passed to full_geocode has a few problems #279

Closed slinkp closed 11 years ago

slinkp commented 11 years ago

This code: https://github.com/openplans/openblock/blob/master/ebpub/ebpub/geocoder/base.py#L498

runs into a few problems when convert_to_block is True. Seems it's been copied from somewhere else but not fully adapted. For easier reference the code is:

#!python
    except InvalidBlockButValidStreet, e:
        # If the exact address couldn't be geocoded, try using the
        # normalized location name.
        if convert_to_block:
            block_name = address_to_block(kwargs['location_name'])
            if block_name != kwargs['location_name']:
                try:
                    result = BlockGeocoder._do_geocode(block_name)
                    result['result']['address'] = block_name
                except InvalidBlockButValidStreet, another_e:
                    pass
                except AmbiguousResult, another_e:
                    pass
        # No joy, return all blocks.
        logger.debug('Invalid block for %r, returning all possible blocks' % query)
        return {'type': 'block', 'result': e.block_list, 'ambiguous': True, 'street_name': e.street_name, 'block_number': e.block_number}

First we need a from ebpub.utils.text import address_to_block before using address_to_block, that's easy.

I think kwargs['location_name'] needs to be replaced with query throughout.

We need an instance of !BlockGeocoder to call _do_geocode

result['result']['address'] = block_name generates KeyError: 'result', I think that's maybe supposed to be result['address'] = block_name

And in that case where we have a result it needs to be returned somehow?

slinkp commented 11 years ago

(In [de54a19747078e230f5a182f94187b70e8bb18e6]) Fixes #286, major breakage in full_geocode(convert_to_block=True), now with some tests ... still needs more coverage

slinkp commented 11 years ago

Thanks for the report, and the analysis - all correct.

slinkp commented 11 years ago

Ticket imported from Trac: http://developer.openblockproject.org/ticket/286 Reported by: kmtracey