csiro-coasts / emsarray

xarray extension that supports EMS model formats
BSD 3-Clause "New" or "Revised" License
13 stars 2 forks source link

Make clip buffer configurable #12

Closed mx-moth closed 1 year ago

mx-moth commented 2 years ago

When making a clip mask, a one cell buffer is included around the clip region. This was included due to the requirements of the original project. This original requirement still exist, but it is not appropriate for all use cases. The buffer should be optional.

Proposal: Add a new buffer: int = 1 parameter to make_clip_mask() and clip() functions. Users who want to disable the buffer can do so with buffer=0. Users who want a larger buffer can set one if required. Negative buffer sizes would raise an error.

Question: Should the default buffer size be zero or one? A default of zero would require changes in existing code which expects a one cell buffer, however a default of no buffer is the least surprising default. This project is in early days, so changing now will be less disruptive than changing later.

cc @frizwi, as someone who needed the original one cell buffer

frizwi commented 2 years ago

I'm all for making a nicer experience for the users so I'd be happy with buffer=0 as the default. All of my clipping use has not made it to prod yet, so if you make this change, I can easily update my code