Add annulus focal kernel

[chase-dwelle]

Addresses #125, #124

Using the circle kernel to extend to an annulus kernel. Implementation excludes the cells on the bound of the inner radius of the annulus.

Also implements the ability to return the raw z-scores from hotspots.

[chase-dwelle]

Thinking the kernel should be "pure" and not include the focal point. Will push a change.

[thuydotm]

Thanks for the PR, I would love to have some tests with this annulus kernel. With the zscore option added to hotspots, I think we should have a separate function for it.

The ongoing stuff with focal is to allow some other kernel shapes (eg: cone, wedge) and also custom kernel. We'd better supply kernel value to a function instead of creating them inside the function, I believe. So basically, the interface should look be something like this:

def _validate_kernel_shape(custom_kernel, shape, radius, outer_radius, inner_radius, ...):
    if (shape == 'circle'  and radius is valid):
        if custom_kernelis not None or outer_radius is not None or inner_radius is not None:
             raise ValueError("""circular kernel must be specified by shape='circle' and a valid `radius`, recieved: ...""")
    # check other kernel shapes
    ....

def create_kernel(cellsize_x, cellsize_y, custom_kernel, shape, radius, outer_radius, inner_radius):
      _validate_kernel_shape()
      if shape == 'circle':
           kernel = _gen_circular_kernel(cellsize_x, cellsize_y, radius)
      elif shape == 'annulus':
           kernel = _gen_annulus_kernel(cellsize_x, cellsize_y, outer_radius, inner_radius)
      ....
      return kernel

def apply(raster, kernel, func):
      ....

[chase-dwelle]

Thanks, will make these changes. Would it make sense to extend the kernel by passing **kwargs considering the expansion of kernel shapes? Could also pass it as a dictionary to make definition and declaration of kernels more explicit.

[thuydotm]

@chase-dwelle with the expansion of kernel shapes, I'm thinking that we can provide a bunch of functions that explicitly tell the shape in their names instead of having 1 function with a long list of args.

def circular_kernel(cellsize_x, cellsize_y, radius):
      # first validate args
      # create and return 2d array of kernel

def annulus_kernel(cellsize_x, cellsize_y, outer_radius, inner_radius):
      # first validate args
      # create and return 2d array of kernel 

def custom_kernel(kernel_values):
     ...

How do you think?

[thuydotm]

Anyway, these are some next steps. We're happy to have this PR merged once having tests updated.

[chase-dwelle]

Added some tests for the kernels and made kernels a required argument to be passed to functions. The data flows go: raster -> cellsize -> kernel -> function/convolution, where cellsize and kernel are returned from functions and are used as arguments instead of passing kernel constructor information to each of the function/convolution methods.

This adds a couple more steps, but also is more straightforward from a code readability standpoint (in my opinion).

I think it might be reasonable to refactor kernel construction to a xrspatial/kernels.py in a future PR as kernel information is going to only grow more. Would that be reasonable @thuydotm ?

[thuydotm]

@chase-dwelle hey the PR looks good and we'll merge it now.

The data flow sounds good. Next step is to remove the function create_kernel. A kernel of any shape can be created directly by calling its corresponding function. It makes sense to assume that user knows which shape they want and which params related.

We should still have kernel functions in focal in my opinion since it's only used for focal stats currently.