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

Feature/centroids as gdf #787

Merged
merged 223 commits into from Apr 11, 2024
Merged

Feature/centroids as gdf #787

merged 223 commits into from Apr 11, 2024

Conversation

chahank
Copy link
Member

@chahank chahank commented Sep 28, 2023

Changes proposed in this PR:

  • Make all the centroids data to be stored in a geodataframe attribute
  • Removes and simplifies a lot of the methods.

This is a DRAFT, and comments are welcome. Not all tests are already running through as many tests are using the centroids class even though they should not. But this will be fixed once we converge to a final result.

This pull request is getting close to review-ready, pending some open discussion points (see below).

PR Author Checklist

PR Reviewer Checklist

@tovogt
Copy link
Collaborator

tovogt commented Feb 14, 2024

I just wanted to drop here for reference the following point:

Centroids coordinates used to be reliably in lat/lon coordinates (epsg4326). Or at least that's what I assumed. Now that they are in a GeoDataFrame with a "serious" crs attribute, we should probably make sure that none of the functions that use centroids just blindly assume that the Centroids.coord attribute is in lat/lon. That applies to trop_cyclone (and tc_rainfield in petals), but it might also apply to other modules.

@idetring
Copy link
Collaborator

Test and Unittests have been updated and complemented to about a coverage of 80%. Former tests in test_vec_ras.py have been incorporated into the test_centr.py mostly as they were, which still results in some doubling of testing ideas.

@emanuel-schmid emanuel-schmid marked this pull request as ready for review April 11, 2024 21:09
@emanuel-schmid emanuel-schmid merged commit 0c9edcb into develop Apr 11, 2024
12 checks passed
@emanuel-schmid emanuel-schmid deleted the feature/centroids_as_gdf branch April 15, 2024 09:33
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.

None yet

9 participants