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

Splitting selection_routines into subfiles #2995

Merged
merged 2 commits into from
Dec 15, 2020

Conversation

matthewturk
Copy link
Member

PR Summary

This is an experiment to try to reduce the complexity and file size of the selection_routines.pyx file. It doesn't make any changes to it other than to split each selector (and the set of boolean selectors as a group) into their own files.

This should make implementing a new selector a bit easier, as it can be templated, and it should also make it much easier if we want to apply templates via DEF and multiple include calls.

PR Checklist

  • pass black --check yt/
  • pass isort . --check --diff
  • pass flake8 yt/
  • pass flynt yt/ --fail-on-change --dry-run -e yt/extern
  • New features are documented, with docstrings and narrative docs
  • Adds a test for any bugs fixed. Adds tests for new features.

Copy link
Member

@neutrinoceros neutrinoceros left a comment

Choose a reason for hiding this comment

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

Left a question but I trust that you did nothing else than cut & paste (+ #include), in which case fire away !

Comment on lines +60 to +101
# cdef np.float64_t *arr[2]
# cdef np.float64_t pos[3]
# cdef np.float64_t H, D, R2, temp
# cdef int i, j, k, n
# cdef int all_under = 1
# cdef int all_over = 1
# cdef int any_radius = 0
# # A moment of explanation (revised):
# # The disk and bounding box collide if any of the following are true:
# # 1) the center of the disk is inside the bounding box
# # 2) any corner of the box lies inside the disk
# # 3) the box spans the plane (!all_under and !all_over) and at least
# # one corner is within the cylindrical radius

# # check if disk center lies inside bbox
# if left_edge[0] <= self.center[0] <= right_edge[0] and \
# left_edge[1] <= self.center[1] <= right_edge[1] and \
# left_edge[2] <= self.center[2] <= right_edge[2] :
# return 1

# # check all corners
# arr[0] = left_edge
# arr[1] = right_edge
# for i in range(2):
# pos[0] = arr[i][0]
# for j in range(2):
# pos[1] = arr[j][1]
# for k in range(2):
# pos[2] = arr[k][2]
# H = D = 0
# for n in range(3):
# temp = self.difference(pos[n], self.center[n], n)
# H += (temp * self.norm_vec[n])
# D += temp*temp
# R2 = (D - H*H)
# if R2 < self.radius2 :
# any_radius = 1
# if fabs(H) < self.height: return 1
# if H < 0: all_over = 0
# if H > 0: all_under = 0
# if all_over == 0 and all_under == 0 and any_radius == 1: return 1
# return 0
Copy link
Member

Choose a reason for hiding this comment

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

any reason we'd want to keep this commented code around ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll try to remove in a future PR!

@neutrinoceros neutrinoceros added the refactor improve readability, maintainability, modularity label Dec 15, 2020
@neutrinoceros neutrinoceros merged commit 384c0e1 into yt-project:master Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor improve readability, maintainability, modularity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants