Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix upcoming sparse change #396

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Fix upcoming sparse change #396

merged 1 commit into from
Oct 31, 2024

Conversation

aulemahal
Copy link
Collaborator

While doing the other PR I stumbled upon an issue with sparse 0.16, which is not yet released. In the SpatialAverager, when the polygon has holes, we compute the weights of the holes and then invert them. The inversion of a COO array also inverts its fill_value. Weights are float, so a fill value of 0.0 gets reverted to -0.0. It seems that up to sparse 0.15.4, this was not a problem but with 0.16, the subsequent concatenation of the weights fails. I was told that this was not a bug : pydata/sparse#802

So here is a fix for that.

Also, I modified the __del__ methods. When the __init__ of either Regridder or SpatialAverager fails before reaching the super().__init__, self.grid_in doesn't exist yet. The AttributeError raised when deleting the object is not raised in the main thread, but emits a warning instead. This avoids that.

@aulemahal aulemahal requested a review from huard October 31, 2024 19:39
Copy link
Contributor

@raphaeldussin raphaeldussin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@raphaeldussin raphaeldussin merged commit c370297 into master Oct 31, 2024
11 checks passed
@raphaeldussin raphaeldussin deleted the fix-sparse-change branch October 31, 2024 19:45
@aulemahal
Copy link
Collaborator Author

@raphaeldussin @huard I was thinking of releasing v0.8.8 soon, so that we can address the xesmf-feedstock issue/pr have a version fully compatible with xarray. Do you see any other PRs or issues we could/should add in before ?

@raphaeldussin
Copy link
Contributor

I think that's a good idea! how are we doing with PR #253 ?
is there closure on the horizon?

@aulemahal
Copy link
Collaborator Author

According to the top comment, it depends on the status of another PR, which is not merged yet!

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