canvasapi icon indicating copy to clipboard operation
canvasapi copied to clipboard

Validate parameter type hints

Open Mike-Nahmias opened this issue 6 years ago • 4 comments

In canvas.py in get_section(), the section param should also have type str listed.

Version: 0.10.0 Python version: 3.7

Pycharm was giving me a type warning when using a string SIS ID to get a section.

Current type: :type section: :class:canvasapi.section.Section or int

Should probably be changed to: :type section: int, str or :class:canvasapi.section.Section

Mike-Nahmias avatar Aug 10 '18 15:08 Mike-Nahmias

Good catch! I'm wondering if we should keep this open for a week or so and see if we can make this broader and include other docstring tweaks. We can go ahead and fix this specific example in the 0.11.0 release, but I'd like to see if anything else is off elsewhere.

I'm not sure if there's a way to scan the parameter types en masse, though. Any thoughts? @Thetwam, how about you?

jessemcbride avatar Aug 10 '18 15:08 jessemcbride

I'm assuming this may only be an issue for methods with the 'use_sis_id' param, since an SIS ID can be a string. The only other issue I can find so far is canvasapi.canvas.get_group() is missing type str for the group param.

Mike-Nahmias avatar Aug 10 '18 15:08 Mike-Nahmias

Also on the topic of type hints, the request methods in canvasapi.requester have the type for params/data listed as dict, but I believe it should be a list of tuples.

Mike-Nahmias avatar Aug 10 '18 16:08 Mike-Nahmias

Another good find. We modified canvasapi.requester.Requester.request a while ago to convert keyword arguments to tuples in order to solve a problem we were having with nested parameters (Canvas expects things like param[][x][y][z] sometimes).

I think those three things (the original issue, the canvas.get_group() find, and this) are reason enough to make this broader. Let's rename this issue to "Validate parameter type hints" or something similar.

jessemcbride avatar Aug 10 '18 16:08 jessemcbride