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.

Sign up for GitHub

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

Jump to bottom

feat: add aggregate_by_weighted_sum to geomapper #1960

Merged
merged 9 commits into from
May 7, 2024
Merged

feat: add aggregate_by_weighted_sum to geomapper #1960

merged 9 commits into from
May 7, 2024

Conversation

dshemetov
Copy link
Contributor

Description

A function that's being duplicated in nwss and nssp indicators.

Changelog

  • update geomap docs and lint
  • add new aggregation function to geomapper

Fixes

@dshemetov dshemetov requested a review from dsweber2 May 6, 2024 22:34
dsweber2
dsweber2 reviewed May 7, 2024
View reviewed changes
Copy link
Contributor

@dsweber2 dsweber2 left a comment

Choose a reason for hiding this comment

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

I would probably also transfer over some of the tests to make sure this function is working as intended.

Also, someone who is not me should probably check the docs.

_delphi_utils_python/delphi_utils/geomap.py Show resolved Hide resolved
@dshemetov dshemetov requested a review from nmdefries May 7, 2024 22:59
@dshemetov
Copy link
Contributor Author

dshemetov commented May 7, 2024 •
edited
Loading

Hey @nmdefries , have you done some geomapper work? Mind looking over the doc changes in ff2d341 ? Nevermind, not too critical, we'll just merge.

@dshemetov dshemetov requested review from dsweber2 and removed request for nmdefries May 7, 2024 23:03
dsweber2
dsweber2 approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@dsweber2 dsweber2 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

tests lgtm

@dshemetov dshemetov merged commit 0c65bb4 into main May 7, 2024
15 checks passed
@dshemetov dshemetov deleted the ds/geomap branch May 7, 2024 23:07
This was referenced Jul 10, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsweber2 dsweber2 dsweber2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dshemetov @dsweber2