daranzolin / rcanvas

R Client for Canvas LMS API
Other
90 stars 43 forks source link

get_discussion_id() errors due to row combine procedure #44

Open coatless opened 4 years ago

coatless commented 4 years ago

While attempting to retrieve discussion components with get_discussion_id(), I'm receiving an error of:

get_discussion_id(discussion_id, object_id)
Error: Argument 5 is a list, must contain atomic vectors

The API call issues is found under get a single topic with structure:

/api/v1/groups/:group_id/discussion_topics/:topic_id

The error is being triggered inside rcanvas:::process_response()

Specifically, when it attempts to bind multiple data frames together, e.g.

https://github.com/daranzolin/rcanvas/blob/7496d66934fd5c5a3f7a8247ff4d8d02aeab4682/R/process_response.R#L20-L26

wsphd commented 4 years ago

James,

The original contributor to the package has moved on.  I too have issues but I can't wait.  It's just easier (in the short-run) to use the a Python API.  Over time, I suppose I can use the reticulate package in R to coordinate things, but it's still at the end of the day a Python script.

And yes to your other question--retrieving full responses to discussions and threads (with author's SIS ID or other) is useful.

Wayne

On 2/5/2020 1:19 PM, James J Balamuta wrote:

While attempting to retrieve discussion components with |get_discussion_id()|, I'm receiving an error of:

get_discussion_id(discussion_id,object_id) Error: Argument 5 is a list,must contain atomic vectors

The API call issues is found under get a single topic https://urldefense.proofpoint.com/v2/url?u=https-3A__canvas.instructure.com_doc_api_discussion-5Ftopics.html-23method.discussion-5Ftopics-5Fapi.show&d=DwMCaQ&c=Oo8bPJf7k7r_cPTz1JF7vEiFxvFRfQtp-j14fFwh71U&r=arUOXBJV185ReiAVcorpVCJACrg7Z_StHteKeAMMoBM&m=5A5Xn3SxvpOBOP9OfNbrE_oOfoywirG8lrh0mFu88Fc&s=p9P55Fsguq7gd6VbBv1M6ftUPad52t6WLx-aBK-Rb40&e= with structure:

|/api/v1/groups/:group_id/discussion_topics/:topic_id |

The error is being triggered inside |rcanvas:::process_response()|

Specifically, when it attempts to bind multiple data frames together, e.g.

https://github.com/daranzolin/rcanvas/blob/7496d66934fd5c5a3f7a8247ff4d8d02aeab4682/R/process_response.R#L20-L26 https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_daranzolin_rcanvas_blob_7496d66934fd5c5a3f7a8247ff4d8d02aeab4682_R_process-5Fresponse.R-23L20-2DL26&d=DwMCaQ&c=Oo8bPJf7k7r_cPTz1JF7vEiFxvFRfQtp-j14fFwh71U&r=arUOXBJV185ReiAVcorpVCJACrg7Z_StHteKeAMMoBM&m=5A5Xn3SxvpOBOP9OfNbrE_oOfoywirG8lrh0mFu88Fc&s=cJJG2KSgBXN8nWceu-H4trrDu7ZC7pIiIvjLk2G_vrw&e=

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_daranzolin_rcanvas_issues_44-3Femail-5Fsource-3Dnotifications-26email-5Ftoken-3DAAN7WWDAYQOUD6ZF6L3MUCTRBMUNZA5CNFSM4KQSHJ72YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4ILKTO6Q&d=DwMCaQ&c=Oo8bPJf7k7r_cPTz1JF7vEiFxvFRfQtp-j14fFwh71U&r=arUOXBJV185ReiAVcorpVCJACrg7Z_StHteKeAMMoBM&m=5A5Xn3SxvpOBOP9OfNbrE_oOfoywirG8lrh0mFu88Fc&s=pyhAc6CIubbfPDE3pA93Lb6POw73hCN_Fz1p-CRxvcA&e=, or unsubscribe https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AAN7WWCVL6MQK7OFW3KSJ2TRBMUNZANCNFSM4KQSHJ7Q&d=DwMCaQ&c=Oo8bPJf7k7r_cPTz1JF7vEiFxvFRfQtp-j14fFwh71U&r=arUOXBJV185ReiAVcorpVCJACrg7Z_StHteKeAMMoBM&m=5A5Xn3SxvpOBOP9OfNbrE_oOfoywirG8lrh0mFu88Fc&s=bl1-iQkF9x0xJ3bR2Q1m0AV4Sz6LEB0NmWWWE4Q55TY&e=.

coatless commented 4 years ago

@wsphd Thanks for the fast response. If that's the case, mind if I fork the package?

I'd like to setup some bindings as UIUC recently started piloting Canvas (https://illinoisedu.instructure.com/)

wsphd commented 4 years ago

James,

I think forking is in order.  David's work (he is also at a University) was key but I think keeping up with Canvas's large and changing API is, frankly, a full-time job in itself.

And Canvas' use is only growing, not shrinking.  Also, some basics from Moodle weren't/aren't in Canvas.  We need to make Canvas better (IMHO).

Wayne

On 2/5/2020 5:01 PM, James J Balamuta wrote:

@wsphd https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_wsphd&d=DwMCaQ&c=Oo8bPJf7k7r_cPTz1JF7vEiFxvFRfQtp-j14fFwh71U&r=arUOXBJV185ReiAVcorpVCJACrg7Z_StHteKeAMMoBM&m=SWpit10fVm2zDTNozx9aEvm6ZjrAHxVO6-HO-_-dg6w&s=koRA1oVWYRyU_eYHZDgBo42A_HQMX-5rkd5nF96JGMc&e= Thanks for the fast response. If that's the case, mind if I fork the package?

I'd like to setup some bindings as UIUC recently started piloting Canvas (https://illinoisedu.instructure.com/ https://urldefense.proofpoint.com/v2/url?u=https-3A__illinoisedu.instructure.com_&d=DwMCaQ&c=Oo8bPJf7k7r_cPTz1JF7vEiFxvFRfQtp-j14fFwh71U&r=arUOXBJV185ReiAVcorpVCJACrg7Z_StHteKeAMMoBM&m=SWpit10fVm2zDTNozx9aEvm6ZjrAHxVO6-HO-_-dg6w&s=95_SsKQza8CZhjgqg7i6IVXqk-d_ARUYRYxyUEKihJg&e=)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_daranzolin_rcanvas_issues_44-3Femail-5Fsource-3Dnotifications-26email-5Ftoken-3DAAN7WWHIZV5KOOQ2DNNJAELRBNONNA5CNFSM4KQSHJ72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK5RMPY-23issuecomment-2D582686271&d=DwMCaQ&c=Oo8bPJf7k7r_cPTz1JF7vEiFxvFRfQtp-j14fFwh71U&r=arUOXBJV185ReiAVcorpVCJACrg7Z_StHteKeAMMoBM&m=SWpit10fVm2zDTNozx9aEvm6ZjrAHxVO6-HO-_-dg6w&s=N2tMRD2JN6JlyPDuf3wtxu9sDttxt6oM0KPLnlqVaFs&e=, or unsubscribe https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AAN7WWAZ2PDQWHGOBO3MQADRBNONNANCNFSM4KQSHJ7Q&d=DwMCaQ&c=Oo8bPJf7k7r_cPTz1JF7vEiFxvFRfQtp-j14fFwh71U&r=arUOXBJV185ReiAVcorpVCJACrg7Z_StHteKeAMMoBM&m=SWpit10fVm2zDTNozx9aEvm6ZjrAHxVO6-HO-_-dg6w&s=8I2kvs-rvTNKIFMzFD2lN4bYAbzptv_hwCSgeaSR5w4&e=.

daranzolin commented 4 years ago

Yes, sorry everyone--rcanvas is no longer relevant to my work, and I'm happy to add maintainers beyond @vanatteveldt. @coatless are you interested? The repo could also hypothetically be transferred.

vanatteveldt commented 4 years ago

Hey guys, yeah sorry for not being very active so far. I had plenty of plans, but teaching (and writing a book) got into my way so far. I would be happy to contribute if someone else take's over the lead as well, otherwise I hope to have more time in the coming month. Whether this is best done as a fork, a transfer, or a github organization I'm not quite sure.

It might also be good to have a discussion somewhere on what the goals/priorities are that everyone has. I would like to get it on CRAN asap as that would make it a lot easier to get working esp. for mac/windows users, and we can think about other calls that we would like to wrap.

bbbruce commented 4 years ago

Please see if the changes I pushed a10c15d just now solved this problem. It appears this occurs because the data returned cannot always be coerced into a data.frame. Please note that I've changed the "setup" in two ways. First, the API token is now more securely stored in your OS's keyring and that setting the domain is a per session setting.