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: result value above 100% for mapping saturation #479

Merged
merged 8 commits into from
Mar 7, 2023
Merged

feat: result value above 100% for mapping saturation #479

merged 8 commits into from
Mar 7, 2023

Conversation

Gigaszi
Copy link
Contributor

@Gigaszi Gigaszi commented Feb 9, 2023 •
edited by hn437

Corresponding issue

Closes #282, #283

New or changed dependencies

  • None

Checklist

  • I have updated my branch to main (e.g. through git rebase main)
  • My code follows the style guide and was checked with pre-commit before committing
  • I have commented my code
  • I have added sufficient unit and integration tests
  • I have updated the CHANGELOG.md

@Gigaszi
Copy link
Contributor Author

Gigaszi commented Feb 9, 2023 •
edited

water_area
forest_area

These are some examples of the plots with a value over 1.

hn437
hn437 requested changes Feb 14, 2023
View reviewed changes
Copy link
Contributor

@hn437 hn437 left a comment

Choose a reason for hiding this comment

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

File: indicator.py
The additions are good, but I would keep the else statement at the end of the elif statements to catch unexpected values should they unexpectedly occure

File: test_indicator_mapping_saturation.py
no further change requested

workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py Show resolved Hide resolved
@matthiasschaub matthiasschaub changed the title MappingSaturation: add classification and plot creation for over 100% results feat: result value above 100% for mapping saturation Mar 7, 2023
matthiasschaub
matthiasschaub previously approved these changes Mar 7, 2023
View reviewed changes
Copy link
Collaborator

@matthiasschaub matthiasschaub left a comment

Choose a reason for hiding this comment

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

Thanks for this addition. I would squash existing commits into one.

@hn437 hn437 merged commit 38ed5dd into main Mar 7, 2023
@hn437 hn437 deleted the saturation_over_100 branch March 7, 2023 14:09
This was referenced Mar 15, 2023
Closed
Closed
matthiasschaub pushed a commit that referenced this pull request Mar 21, 2023
@Gigaszi @hn437 @matthiasschaub
feat: result value above 100% for mapping saturation (#479)
1ee7716 
* feat: add classification and plot creation for over 100% results

* feat: new indicator classification

* add: revert else-statement

---------

Co-authored-by: Levi Szamek <levi.szamek@stud.uni-heidelberg.de>
Co-authored-by: Jonas <36933111+hn437@users.noreply.github.com>
matthiasschaub pushed a commit that referenced this pull request Mar 21, 2023
@Gigaszi @hn437 @matthiasschaub
feat: result value above 100% for mapping saturation (#479)
d5999ac 
* feat: add classification and plot creation for over 100% results

* feat: new indicator classification

* add: revert else-statement

---------

Co-authored-by: Levi Szamek <levi.szamek@stud.uni-heidelberg.de>
Co-authored-by: Jonas <36933111+hn437@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hn437 hn437 hn437 approved these changes

@matthiasschaub matthiasschaub matthiasschaub left review comments

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

Successfully merging this pull request may close these issues.

Result of Mapping Saturation indicator over 10
3 participants
@Gigaszi @hn437 @matthiasschaub