-
Notifications
You must be signed in to change notification settings - Fork 18
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
Rename bounding_box to cell_vectors consistently #127
Conversation
I disagree with this change. |
Fine with me as long as it's consistent in the code base. @jgreener64 @rkurchin others thoughts ? |
It seems that the core point here is that we should be consistent about what we call this thing, which I definitely agree with. I personally do not have strong feelings about which particular name we go with. |
No opinions beyond consistency. |
not sure what to say here, I stand by what I said. google "bounding box" and "cell vectors" and see what you get. Bounding box is a concept from image processing, while cell vectors immediately brings up links to Wikipedia Unit Cell. If you all disagree with me and decide you prefer to stick with the existing terminology, then not much I can do. If you want to consider changing the terminology, then the natural thing to do would be to deprecate the |
I would prefer |
To clarify I am fine with that change as long as the same term is used throughout the codebase. |
Same here. If no one expresses clear preference in the other direction, I'd suggest we change everything to |
Ok, this will now completely remove How do we feel about the plural form here. I used it because of our previous discussion. But now I note that I'm not too sure about this. Singular is a bit strange here, too. What are your opinions ? |
I think the plural form Also just pointing out that this is technically breaking as |
Technically we can use Using the plural is not inconsistent because I strongly support plural. (and yes to the last post - this is breaking and will have a deprecation cycle.) |
I'm good with this being merged once the docs issue is fixed. I would suggest we add a minor version bump here too though so we can start having the deprecation warnings etc. |
Sure, I agree. Should get to it later today. |
Unless there are objections I'll merge this tomorrow. |
Renamed occurrences of cell_vectors to bounding_box for consistency.