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

Use same code style for inverse_distance_weighted.py and nearest_neighbor.py #924

Closed
ahijevyc opened this issue Aug 30, 2024 · 5 comments
Closed
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@ahijevyc
Copy link
Collaborator

Under uxarray.remap, inverse_distance_weighted.py and nearest_neighbor.py have a lot of the same code but have different documentation style. Their formatting styles should probably be brought back into agreement.

There is a lot of repeated code also. Perhaps that can be just in one place and reused.

I put in a pull request #923 for a neighborhood filter apply_func that started with the code in these functions as a template, so there was a lot of copying and pasting. If the shared code was in one place it could serve 3 different functions, inverse_distance_weighted.py and nearest_neighbor.py and the potential new function apply_func.

@github-project-automation github-project-automation bot moved this to 📚 Backlog in UXarray Development Aug 30, 2024
@ahijevyc ahijevyc added good first issue Good for newcomers documentation Improvements or additions to documentation and removed good first issue Good for newcomers labels Aug 30, 2024
@philipc2 philipc2 self-assigned this Aug 30, 2024
@aaronzedwick
Copy link
Member

I like this idea a lot, especially coming up with a condensed version of the reused functions. I can tackle this most likely this week.

@aaronzedwick
Copy link
Member

aaronzedwick commented Sep 27, 2024

Thinking about this more, I don't know what code we could reuse as a function. Although similar, the nearest neighbor and inverse distance functions use completely different parameters for calculating things. So the things we could reuse would only save a few extra lines, and just add an extra file to the code base without too much reason.

@ahijevyc
Copy link
Collaborator Author

ahijevyc commented Sep 27, 2024

Thanks for looking at this. I was thinking of the similarity of lines 63-138 in inverse_distance_weighted.py and lines 45-128 in nearest_neighbor.py. There are differences in the order of elif-clauses and the names of some variables, but otherwise, they are functionally equivalent. In nearest_neighbor.py the variables are named cart_x, cart_y, cart_z while in inverse_distance_weighted.py they are named x, y, z.

Plus, there a lot of similarities in the function descriptions but one has argument type hints (nearest_neighbor.py) and the other has no type hints inverse_distance_weighted.py. The Parameters: sections also have single-character or whitespace differences at the end of lines.

I found this comparison was easy using gvim -d or gvimdiff.

@aaronzedwick
Copy link
Member

Thanks for looking at this. I was thinking of the similarity of lines 63-138 in inverse_distance_weighted.py and lines 45-128 in nearest_neighbor.py. There differences in the order of elif-clauses and the names of some variables, but otherwise, they are functionally equivalent. In nearest_neighbor.py the variables are named cart_x, cart_y, cart_z while in inverse_distance_weighted.py they are named x, y, z.

Plus, there a lot of similarities in the function descriptions but one has argument type hints (nearest_neighbor.py) and the other has no type hints inverse_distance_weighted.py. The Parameters: sections also have single-character or whitespace differences at the end of lines.

I found this comparison was easy using gvim -d or gvimdiff.

Gotcha, yeah I can make a function out of those specific lines. I have a PR up, I'll tag you for a review once it's ready.

@aaronzedwick
Copy link
Member

Completed in PR #974

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: ✅ Done
Development

No branches or pull requests

3 participants