Skip to content

Fix IndexError in plot_shear_by_sector with custom direction_bin_labels - #643

Open
Jah-yee wants to merge 1 commit into
brightwind-dev:masterfrom
Jah-yee:fix/shear-by-sector-custom-labels
Open

Jah-yee wants to merge 1 commit into
brightwind-dev:masterfrom
Jah-yee:fix/shear-by-sector-custom-labels

Conversation

@Jah-yee

@Jah-yee Jah-yee commented Oct 3, 2026

Copy link
Copy Markdown

Summary

Fix IndexError in plot_shear_by_sector() when custom direction_bin_labels are used with Shear.BySector().

Problem

The function uses a regex to extract numeric bin edges from index labels. When labels are non-numeric (custom text labels), the regex returns an empty list, causing IndexError at index [0].

Solution

  1. Try extracting numbers from the index label first (works for default labels like '0.0-90.0')
  2. If fewer than 2 numbers found, fall back to using direction_bin_array (already available in Shear.BySector)
  3. Added direction_bin_array parameter to plot_shear_by_sector() and updated Shear.BySector to pass it through
  4. Added a clear ValueError with helpful message as last resort

Changes

  • brightwind/analyse/plot.py: Added direction_bin_array=None parameter; updated bin_edges extraction with fallback logic
  • brightwind/analyse/shear.py: Pass direction_bin_array to both plot_shear_by_sector() calls

Fixes #636

When custom direction_bin_labels (e.g. ['northerly', 'easterly', ...])
are passed to Shear.BySector, plot_shear_by_sector() fails with IndexError
because re.findall() cannot extract numeric bin edges from non-numeric
label strings.

Fix: Try extracting numbers from index label first; if that fails (fewer
than 2 numbers found), use direction_bin_array to compute bin edges.
Added direction_bin_array parameter to plot_shear_by_sector() and updated
Shear.BySector to pass it through.

Fixes brightwind-dev#636
@Jah-yee

Jah-yee commented Oct 25, 2026

Copy link
Copy Markdown
Author

Hi — I want to double-check one detail of the plot_shear_by_sector change in brightwind/analyse/plot.py. The previous code did re.findall(r"[-+]?\d*\.\d+|\d+", wind_rose_data.index[i])[0], which raises IndexError when a wind-rose sector label contains no number, and fails on [1] when the label contains only one. Your patch guards the lookup with len(nums) >= 2 and falls back to the new direction_bin_array argument, which matches the failure mode I hit in brightwind/analyse/shear.py. Two things so I can align with your intent: (1) should direction_bin_array stay optional so existing callers of plot_shear_by_sector keep working unchanged, or would you prefer it keyword-only? (2) is there a test module under brightwind/tests where you want a regression case for a single-number sector label? I can adjust the diff to whatever shape you prefer.

@Jah-yee

Jah-yee commented Nov 1, 2026

Copy link
Copy Markdown
Author

Following up on the plot_shear_by_sector() IndexError fix, since this has been open for a while and I want to make sure the change is still what you would want to merge.

What the branch changes, concretely:

  • brightwind/analyse/plot.py — plot_shear_by_sector() no longer assumes the index labels contain numbers. It first tries to pull the numeric bin edges out of the label with the regex, and only when that yields fewer than two numbers does it fall back to the direction_bin_array that Shear.BySector() already computes internally.
  • brightwind/analyse/shear.py — Shear.BySector() now passes its direction_bin_array through to plot_shear_by_sector() so the fallback has the real edge values available.

The original failure was that with custom text labels such as "calm" or "gale" the regex returned an empty list, and the code then indexed [0] on it and raised IndexError. The fallback path plus the explicit ValueError with a message naming the offending label turns that into an actionable error instead of a crash.

One thing worth flagging for review: the fallback relies on direction_bin_array being the same edges the labels were built from, so if a caller passes labels that are custom but not derived from the sector bins, the labels and the plotted edges could disagree. I kept the strict ValueError in that case rather than silently plotting against mismatched edges.

If you would prefer a different resolution order (for example, always using direction_bin_array and dropping the regex path entirely), say so and I will rework it. Happy to also add a regression test for the custom-label case if the repository has a test directory for analyse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Shear.BySector] IndexError when direction_bin_labels is given

1 participant