search

CartoDB / crankshaft

CARTO Spatial Analysis extension for PostgreSQL
BSD 3-Clause "New" or "Revised" License
54 stars 20 forks source link

catch empty return values and error on them #157

Closed andy-esch closed 6 years ago

andy-esch commented 7 years ago

This fixes a problem where empty lists of data were passed to algorithms. Before the data analysis provider was written, the default was to return [(None, None, ...)] (an empty tuple with the correct number of columns). With this PR, I'm choosing a noisy error if the input data has so many null that the data cannot be analyzed.

@talos, could you do a CR, please? Specifically, I'm interested in your opinion of the verify_data data function, and whether I should be raising an exception and handling it in the try, except block instead.

closes #143, closes #156

talos commented 7 years ago

Takin' a look!

andy-esch commented 7 years ago

Thanks for the review @talos ! For 1. above, I may not have said it very clearly, but this PR's goal is to get rid of returning nulls in favor of terminating the flow, so all good here.

andy-esch commented 7 years ago

@talos, I added basic usage of decorators here that reduces the redundancy and increases the maintainability of the code :) Please take a look and let me know what you think. Do you still think that I should take a look at functools?

andy-esch commented 7 years ago

It looks like the functools.wrap just carries over the function metadata? In this case, that doesn't seem like an important property.

andy-esch commented 7 years ago

@talos, anything else before I hand this PR over?

talos commented 7 years ago

👍