Skip to content

export Base.LinAlg.chksquare #292

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

Closed
tpapp opened this issue Dec 13, 2015 · 13 comments · Fixed by JuliaLang/julia#14601
Closed

export Base.LinAlg.chksquare #292

tpapp opened this issue Dec 13, 2015 · 13 comments · Fixed by JuliaLang/julia#14601

Comments

@tpapp
Copy link
Contributor

tpapp commented Dec 13, 2015

Rationale: checking that a matrix is square is a pretty common operation. While chksquare is a minor utility function, it is well-designed, useful, and allows standard error reporting. Without a common function, libraries end up implementing their own solutions or explicitly spell out something like

n, m = size(A)
if n != m
    throw(ArgumentError("matrix is not square"))

or one of the countless variations. Standardizing this outside Base.LinAlg would be useful.

@andreasnoack
Copy link
Member

I gree that it would be a useful function to expot, but I don't like the chk abbreviation though. Maybe rename to checksquare, write documentation and export in a PR would be the way to proceed.

@tpapp
Copy link
Contributor Author

tpapp commented Dec 15, 2015

I will try submitting a PR, as I mean to learn the workflow anyway and such a minor change would be a good place to start.

@tkelman
Copy link

tkelman commented Dec 15, 2015

This could be used elsewhere without exporting it, just by qualifying it with the module name. I'd be in favor of renaming and documenting it, but not exporting such a trivial thing.

@KristofferC
Copy link
Member

If the name is changed it should be deprecated even though it is a non exported function. It is likely used in many places out of Base.

@StefanKarpinski
Copy link
Member

How about renaming it to checksqr? :trollface:

@tpapp
Copy link
Contributor Author

tpapp commented Dec 15, 2015

How would I note in the documentation that it is not exported? Is there a convention, eg using the full form Base.LinAlg.checksquare?

@tkelman
Copy link

tkelman commented Dec 15, 2015

Or just LinAlg.checksquare since the LinAlg module is exported. That's the convention used for Profile and a few other places, yes.

@jiahao
Copy link
Member

jiahao commented Dec 15, 2015

To clarify, chksquare does more than just check if the matrix is square. If it is square, it also returns the dimension of the matrix, which is an extremely common thing to do. Perhaps we can come up with a name that reflects its return value too.

@tpapp
Copy link
Contributor Author

tpapp commented Dec 16, 2015

What about checked_square_size? A bit long, but reflects the fact that it returns a dimension, and the argument has been checked to be square.

@andreasnoack
Copy link
Member

I think checksquare is fine if we document the behavior and give an example.

@StefanKarpinski
Copy link
Member

+1 for checksquare.

@tpapp
Copy link
Contributor Author

tpapp commented Jan 8, 2016

I just submitted PR JuliaLang/julia#14601, which should close this. Thanks for all the help.

@KristofferC KristofferC transferred this issue from JuliaLang/julia Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants