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

Unclear documentation of checkbounds(::Bool, ...) #15671

Closed
eschnett opened this issue Mar 29, 2016 · 4 comments
Closed

Unclear documentation of checkbounds(::Bool, ...) #15671

eschnett opened this issue Mar 29, 2016 · 4 comments
Labels
docs This change adds or pertains to documentation

Comments

@eschnett
Copy link
Contributor

This function has the documentation

  checkbounds(::Type{Bool}, dimlength::Integer, index)

  Return a Bool describing if the given index is within the bounds of the
  given dimension length. Custom types that would like to behave as indices
  for all arrays can extend this method in order to provide a specialized
  bounds checking implementation.

I don't understand the term "dimension length".

I also don't understand how this helps defining a new type that can be used as index for all arrays -- wouldn't that require re-defining all array types' getindex functions?

Furthermore, there seems to be a function function checkbounds(::Type{Bool}, sz::Dims, I...) that is exported, but not documented.

@mbauman
Copy link
Sponsor Member

mbauman commented Mar 29, 2016

A dimension length is like size(A, 1). I'm definitely open to better terminology here. It's just checking to see if all(1 .<= index .<= dimlength). You're right, it doesn't allow any index type, but it does allow custom Reals and AbstractArrays to change the behavior. It's notably used by DataArrays and NullableArrays to deal with missing data.

Note that this needs get re-worked in order to support cartesian indices: #15449 (comment).

@eschnett
Copy link
Contributor Author

"index range"? "size of dimension"?

Would this work?

  checkbounds(::Type{Bool}, maxsize::Integer, index)

  Return a Bool describing whether the given index is in the range `1:maxsize`.
  Note that `index` can not only be an integer, but also e.g. a floating-point number,
  a range, or a placeholder like `:` (Colon).

  Custom types that would like to behave as indices
  for all arrays can extend this method in order to provide a specialized
  bounds checking implementation.

@mbauman
Copy link
Sponsor Member

mbauman commented Mar 30, 2016

Sure, that'd be fine, but I don't think I'd call out floating point indexes.

@kshyatt kshyatt added backport pending 0.4 docs This change adds or pertains to documentation and removed backport pending 0.4 labels Jan 26, 2017
@kshyatt
Copy link
Contributor

kshyatt commented Jan 26, 2017

The docstring for this function seems to have changed substantially so I'm going to close. Feel free to reopen if it's still unclear.

@kshyatt kshyatt closed this as completed Jan 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

No branches or pull requests

3 participants