-
-
Notifications
You must be signed in to change notification settings - Fork 206
fix: get_edge_ids()
error for 2x2 matrices
#1798
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
base: main
Are you sure you want to change the base?
Conversation
Current Aviator status
This pull request is currently open (not queued). How to mergeTo merge this PR, comment
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
I see that only three packages are affected: corpustools, mwcsr and SEMID. One of the results also hints at a misuse from igraph itself. I still think we should break cleanly and perhaps improve the error message. |
Another alternative: disallow matrices altogether, ask users to convert matrices with |
Fine by me, lets find a way in this PR |
get_edge_ids()
throws warning instead of error for 2x2 matricesget_edge_ids()
error for 2x2 matrices
Let's await revdepchecks in #1793. |
"2.1.5", | ||
"get_edge_ids(vp = 'is not allowed to be a 2 times 2 matrix')" | ||
"get_edge_ids(vp = 'is not allowed to be a 2 times 2 matrix')", | ||
details = c("For backward compatibility, edges are interpreted columnwise.", "convert to a data.frame with two columns and one row per edge.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second message (one row per edge) is clearer. Could the two be combined? My suggestion below is very bad though.
details = c("For backward compatibility, edges are interpreted columnwise.", "convert to a data.frame with two columns and one row per edge.") | |
details = c("For backward compatibility, it is assumed that each row is an edge, from the first column to the second column.", "convert to a data.frame with two columns and one row per edge.") |
The function now throws a warning and indicates how the matrix is interpreted for backward compatibility