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

132 add simple IDW #160

Merged
merged 14 commits into from
Sep 7, 2023
Merged

132 add simple IDW #160

merged 14 commits into from
Sep 7, 2023

Conversation

Mtk112
Copy link
Contributor

@Mtk112 Mtk112 commented Jul 3, 2023 •
edited
Loading

Add simple inverse distance weighting interpolation.

@Mtk112
Interpolate IDW
c5a76ff 
v1 IDW interpolation - WORK IN PROGRESS
@Mtk112
Merge branch 'master' into 132-interpolate-vector
02fbd07
@Mtk112
IDW v2
4722d85 
WORK IN PROGRESS, Tests need to be implemented / fixed.
@Mtk112
Update idw_interpolation.py
5e48c0c 
Work in progress.
@Mtk112
Fixes, tests
77fce85
@Mtk112
Update
a9f471b
@Mtk112 Mtk112 self-assigned this Jul 3, 2023
@Mtk112
Fix pre-commit errors
2d6db04
@Mtk112
Fixes
ff33a1b 
Renamed the function to simple_idw. Fixed tests and notebook. Cleaned some code.
@Mtk112 Mtk112 marked this pull request as ready for review July 6, 2023 12:53
@Mtk112 Mtk112 changed the title 132 interpolate vector 132 add simple IDW Jul 6, 2023
@Mtk112 Mtk112 linked an issue Jul 6, 2023 that may be closed by this pull request
Add simple IDW #132
Closed
msorvoja
msorvoja reviewed Jul 25, 2023
View reviewed changes
Copy link
Collaborator

@msorvoja msorvoja 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! Left a few comments/suggestions.

eis_toolkit/vector_processing/simple_idw.py Outdated Show resolved Hide resolved
eis_toolkit/vector_processing/simple_idw.py Outdated Show resolved Hide resolved
eis_toolkit/vector_processing/simple_idw.py Outdated Show resolved Hide resolved
eis_toolkit/vector_processing/simple_idw.py Outdated Show resolved Hide resolved
eis_toolkit/vector_processing/simple_idw.py Outdated Show resolved Hide resolved
eis_toolkit/vector_processing/simple_idw.py Show resolved Hide resolved
tests/vector_processing/test_simple_idw.py Outdated Show resolved Hide resolved
@Mtk112
Update
c56c491 
Update based on review comments.
msorvoja
msorvoja requested changes Aug 10, 2023
View reviewed changes
Copy link
Collaborator

@msorvoja msorvoja 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.

Hi,
After looking at the code again, I noticed a couple of things I'd like you to check/fix.

eis_toolkit/vector_processing/simple_idw.py Outdated Show resolved Hide resolved
eis_toolkit/vector_processing/simple_idw.py Outdated Show resolved Hide resolved
eis_toolkit/vector_processing/simple_idw.py Outdated Show resolved Hide resolved
@Mtk112
Update
22a45ad 
Fixes based on comments.
nmaarnio
nmaarnio reviewed Sep 7, 2023
View reviewed changes
Copy link
Collaborator

@nmaarnio nmaarnio 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.

Looks good! I am still not sure why there is a slight difference between the output of this algorithm and QGIS IDW, but as the difference seems small enough, I think we should get this merged and review this code once more at a later point. Before merging, can you remove the unneeded file changes (a change in pca_plot_testing.ipynb and addition of desktop.ini) @Mtk112

EDIT: I see you're on a vacation and remove the .ini file myself and proceed to merge

@nmaarnio nmaarnio merged commit 603adc2 into master Sep 7, 2023
2 of 5 checks passed
@nmaarnio nmaarnio deleted the 132-interpolate-vector branch September 7, 2023 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msorvoja msorvoja msorvoja requested changes

@nmaarnio nmaarnio nmaarnio approved these changes

Assignees

@Mtk112 Mtk112

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add simple IDW
3 participants
@Mtk112 @nmaarnio @msorvoja