Skip to content

fix: raise clear error when window_size is too large#1152

Open
suhr25 wants to merge 2 commits intomalariagen:masterfrom
suhr25:fix/fst-gwss-empty-window-guard
Open

fix: raise clear error when window_size is too large#1152
suhr25 wants to merge 2 commits intomalariagen:masterfrom
suhr25:fix/fst-gwss-empty-window-guard

Conversation

@suhr25
Copy link
Contributor

@suhr25 suhr25 commented Mar 18, 2026

SUMMARY

Fixes a crash when window_size is too large in Fst GWSS by raising a clear error instead of returning empty data.


FIX

Before

x = allel.moving_statistic(pos, statistic=np.mean, size=window_size)
return dict(x=x, fst=fst)

After

x = allel.moving_statistic(pos, statistic=np.mean, size=window_size)

if len(x) == 0:
    raise ValueError("window_size is larger than available SNPs")

return dict(x=x, fst=fst)

VERIFICATION

Using a large window_size on a small region used to crash with an IndexError. Now it raises a clear ValueError. A test confirms this behavior.

…gwss

Signed-off-by: Suhrid Marwah <suhridmarwah07@gmail.com>
@suhr25
Copy link
Contributor Author

suhr25 commented Mar 18, 2026

Hi @jonbrenas,
Kindly take a look at this PR.
Thanks!

@jonbrenas
Copy link
Collaborator

Hi @suhr25, would there be a way to instead raise a warning and modify the window_size so that a result is returned?

Replace hard error when window_size exceeds available SNPs with a warning,
and automatically adjust window_size to the maximum valid value so that
computation can proceed.

Signed-off-by: Suhrid Marwah <suhridmarwah07@gmail.com>
@suhr25 suhr25 force-pushed the fix/fst-gwss-empty-window-guard branch from 1b9797e to a05bab0 Compare March 19, 2026 06:09
@suhr25
Copy link
Contributor Author

suhr25 commented Mar 19, 2026

Thanks for the suggestion! @jonbrenas
I have updated the behavior to issue a warning and automatically adjust window_size to the available SNP count so the computation can proceed. Also updated tests and documentation accordingly.

Let me know if you would prefer any different handling.

@suhr25
Copy link
Contributor Author

suhr25 commented Mar 20, 2026

Hi @jonbrenas,
Kindly take a look,Thanks!

@jonbrenas
Copy link
Collaborator

Thanks @suhr25,

Because we want to have a moving statistics, the window size would need to be a factor smaller than the number of SNPs (let's say, 10 by default but it should be a parameter). The window size should thus be adjusted, unless it falls under a given number (again it should be a parameter with a default value say 1 000), in which your original error message probably becomes the solution.

@suhr25
Copy link
Contributor Author

suhr25 commented Mar 20, 2026

Sure thanks,
I will look into it.

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.

2 participants