From 62816518624fe04cdd73e77c9128ad7d545d8e37 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Thu, 25 Jun 2026 11:32:57 -0700 Subject: [PATCH 1/2] Document num_sample=None in natural_breaks docstring (#3501) natural_breaks accepts num_sample=None (use all data) like the other classifiers, but its docstring only described it as int. Bring the parameter wording in line with quantile/maximum_breaks/percentiles. --- xrspatial/classify.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xrspatial/classify.py b/xrspatial/classify.py index d3261a5b9..5bedc21ac 100644 --- a/xrspatial/classify.py +++ b/xrspatial/classify.py @@ -842,12 +842,14 @@ def natural_breaks(agg: xr.DataArray, of values to be reclassified. k : int, default=5 Number of classes to be produced. - num_sample : int, default=20000 + num_sample : int or None, default=20000 Number of sample data points used to fit the model. Natural Breaks (Jenks) classification is indeed O(n²) complexity, where n is the total number of data points, i.e: `agg.size` When n is large, we should fit the model on a small sub-sample of the data instead of using the whole dataset. + ``None`` means use all data (safe for numpy/cupy, + automatically capped for dask). name : str, default='natural_breaks' Name of output aggregate. From ff9b8c47ccce5292892a3c5f96f2517dfbd078e3 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Thu, 25 Jun 2026 12:58:08 -0700 Subject: [PATCH 2/2] Reword natural_breaks num_sample=None note to fit the Jenks path The first pass copied the percentile classifiers' '(safe for numpy/cupy, automatically capped for dask)' wording. For natural_breaks None fits Jenks on all data, which is the O(n^2) case the docstring warns about and which raises MemoryError past the half-RAM guard, so 'safe' was wrong. Describe the actual cost instead. Addresses review on #3505. --- xrspatial/classify.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/xrspatial/classify.py b/xrspatial/classify.py index 5bedc21ac..b085a8488 100644 --- a/xrspatial/classify.py +++ b/xrspatial/classify.py @@ -848,8 +848,11 @@ def natural_breaks(agg: xr.DataArray, where n is the total number of data points, i.e: `agg.size` When n is large, we should fit the model on a small sub-sample of the data instead of using the whole dataset. - ``None`` means use all data (safe for numpy/cupy, - automatically capped for dask). + ``None`` means fit on all data instead of a sub-sample. That is + the full O(n²) case described above, so it may be slow and raises + ``MemoryError`` if the Jenks matrices would exceed half of the + available RAM. For dask the full sample is drawn lazily via + indexed access. name : str, default='natural_breaks' Name of output aggregate.