Repository: ucfopen/canvasapi

Issue:
Accept path-like objects for file uploads
Under `canvasapi` 0.7.0, `canvasapi.upload.Uploader.__init__` allows two ways to identify the file to be uploaded: it may be given as a string filename, or as an already-open file object. However, there are other ways to describe paths to files besides just strings: what the Python documentation calls “[path-like objects](https://docs.python.org/3/glossary.html#term-path-like-object).” For example:

- From Python 3.4 onward, the [`pathlib`](https://docs.python.org/3/library/pathlib.html#pathlib.PurePath) module provides a rich API for manipulating filesystem paths as path-like objects rather than as strings.

- From Python 3.6 onward, the [`os.DirEntry`](https://docs.python.org/3/library/os.html#os.DirEntry) objects returned by [`os.scandir`](https://docs.python.org/3/library/os.html#os.scandir) are path-like objects.

As far as I can tell, the `Uploader` class will not behave correctly if constructed with a path-like object as its `file` argument. This should be improved so that path-like objects are treated like openable filenames, rather than being assumed to be already-open files.

<hr/>

Python 3.6 added several library features to make it easier to write code that transparently handles both string paths as well as path-like objects:

- A new [`os.PathLike`](https://docs.python.org/3/library/os.html#os.PathLike) abstract base class makes it possible to recognize path-like objects in a generic way, rather than checking for specific concrete classes.

- A new [`os.fspath`](https://docs.python.org/3/library/os.html#os.fspath) function returns the string representation of a path-like object, or simply returns its argument unchanged if that argument is already a string.

- Improvements to [`os.open`](https://docs.python.org/3/library/os.html#os.open) and other standard library functions allow them to accept either path-like objects or strings as arguments.

The Python 3.6 release notes include [more information on how to use these features](https://docs.python.org/3/whatsnew/3.6.html#pep-519-adding-a-file-system-path-protocol).

Unfortunately, those Python 3.6 improvements still leave a gap: Python 3.4 and 3.5 have the `pathlib` module, but do not have `os.PathLike`, `os.fspath`, the improvements to `os.open`, etc. Supporting path-like objects in these earlier Python releases may be inconvenient, or perhaps not worth doing at all.

Discussion:
theunkn0wn1: Ok. strangely this works in the test suite but not in my downstream (fails in an actual upload). It appears the tests I added were not sufficient. I recommend not merging this yet til I figure out how to properly test this & fix the bug.
theunkn0wn1: ![image](https://user-images.githubusercontent.com/3110986/106682077-9b3dd880-6576-11eb-9546-dc97544725e6.png)
works in downstream now.
It appears the tests were accurate, and i had a stale version of this project in the test environment leading to my last message :facepalm: 
coveralls: 
[![Coverage Status](https://coveralls.io/builds/38184606/badge)](https://coveralls.io/builds/38184606)

Coverage decreased (-0.03%) to 99.972% when pulling **73c8193b5bdce1b94b142aaa20f8358957b0637b on theunkn0wn1:issue/97-accept-path-like-objects-for-file-uploads** into **9640fa88c6ae1369d9c22d03b967c630dfac1c29 on ucfopen:develop**.

jessemcbride: Our coverage drops here because of the `isinstance(file, Path)` case, but I think we can just remove the condition based on the current public API. @theunkn0wn1 Does that make sense, or are we missing something?
theunkn0wn1: Sounds logical to me. pushed a commit to address that point.

Patch:
diff --git a/canvasapi/assignment.py b/canvasapi/assignment.py
index 82268f2..52253b6 100644
--- a/canvasapi/assignment.py
+++ b/canvasapi/assignment.py
@@ -4,7 +4,7 @@ from canvasapi.paginated_list import PaginatedList
 from canvasapi.peer_review import PeerReview
 from canvasapi.progress import Progress
 from canvasapi.submission import Submission
-from canvasapi.upload import Uploader
+from canvasapi.upload import FileOrPathLike, Uploader
 from canvasapi.user import User, UserDisplay
 from canvasapi.util import combine_kwargs, obj_or_id
 
@@ -318,7 +318,7 @@ class Assignment(CanvasObject):
 
         return Submission(self._requester, response_json)
 
-    def upload_to_submission(self, file, user="self", **kwargs):
+    def upload_to_submission(self, file: FileOrPathLike, user="self", **kwargs):
         """
         Upload a file to a submission.
 
@@ -327,7 +327,7 @@ class Assignment(CanvasObject):
         <https://canvas.instructure.com/doc/api/submissions.html#method.submissions_api.create_file>`_
 
         :param file: The file or path of the file to upload.
-        :type file: file or str
+        :type file: FileLike
         :param user: The object or ID of the related user, or 'self' for the
             current user. Defaults to 'self'.
         :type user: :class:`canvasapi.user.User`, int, or str
diff --git a/canvasapi/course.py b/canvasapi/course.py
index 34512f6..fcb97f0 100644
--- a/canvasapi/course.py
+++ b/canvasapi/course.py
@@ -28,7 +28,7 @@ from canvasapi.rubric import Rubric, RubricAssociation
 from canvasapi.submission import GroupedSubmission, Submission
 from canvasapi.tab import Tab
 from canvasapi.todo import Todo
-from canvasapi.upload import Uploader
+from canvasapi.upload import FileOrPathLike, Uploader
 from canvasapi.usage_rights import UsageRights
 from canvasapi.util import (
     combine_kwargs,
@@ -2621,7 +2621,7 @@ class Course(CanvasObject):
         )
         return response.json()
 
-    def upload(self, file, **kwargs):
+    def upload(self, file: FileOrPathLike, **kwargs):
         """
         Upload a file to this course.
 
diff --git a/canvasapi/folder.py b/canvasapi/folder.py
index 0b27083..8ee6b36 100644
--- a/canvasapi/folder.py
+++ b/canvasapi/folder.py
@@ -1,6 +1,6 @@
 from canvasapi.canvas_object import CanvasObject
 from canvasapi.paginated_list import PaginatedList
-from canvasapi.upload import Uploader
+from canvasapi.upload import FileOrPathLike, Uploader
 from canvasapi.util import combine_kwargs, obj_or_id
 
 
@@ -119,7 +119,7 @@ class Folder(CanvasObject):
 
         return Folder(self._requester, response.json())
 
-    def upload(self, file, **kwargs):
+    def upload(self, file: FileOrPathLike, **kwargs):
         """
         Upload a file to this folder.
 
diff --git a/canvasapi/group.py b/canvasapi/group.py
index 2812da6..1acb666 100644
--- a/canvasapi/group.py
+++ b/canvasapi/group.py
@@ -6,6 +6,7 @@ from canvasapi.folder import Folder
 from canvasapi.license import License
 from canvasapi.paginated_list import PaginatedList
 from canvasapi.tab import Tab
+from canvasapi.upload import FileOrPathLike, Uploader
 from canvasapi.usage_rights import UsageRights
 from canvasapi.util import combine_kwargs, is_multivalued, obj_or_id
 
@@ -966,7 +967,7 @@ class Group(CanvasObject):
         )
         return GroupMembership(self._requester, response.json())
 
-    def upload(self, file, **kwargs):
+    def upload(self, file: FileOrPathLike, **kwargs):
         """
         Upload a file to the group.
         Only those with the 'Manage Files' permission on a group can upload files to the group.
@@ -983,7 +984,6 @@ class Group(CanvasObject):
                     and the JSON response from the API.
         :rtype: tuple
         """
-        from canvasapi.upload import Uploader
 
         return Uploader(
             self._requester, "groups/{}/files".format(self.id), file, **kwargs
diff --git a/canvasapi/submission.py b/canvasapi/submission.py
index 390789c..45a5348 100644
--- a/canvasapi/submission.py
+++ b/canvasapi/submission.py
@@ -1,7 +1,7 @@
 from canvasapi.canvas_object import CanvasObject
 from canvasapi.paginated_list import PaginatedList
 from canvasapi.peer_review import PeerReview
-from canvasapi.upload import Uploader
+from canvasapi.upload import FileOrPathLike, Uploader
 from canvasapi.util import combine_kwargs, obj_or_id
 
 
@@ -143,7 +143,7 @@ class Submission(CanvasObject):
         )
         return response.status_code == 204
 
-    def upload_comment(self, file, **kwargs):
+    def upload_comment(self, file: FileOrPathLike, **kwargs):
         """
         Upload a file to attach to this submission as a comment.
 
diff --git a/canvasapi/upload.py b/canvasapi/upload.py
index e0a68fc..3767cc8 100644
--- a/canvasapi/upload.py
+++ b/canvasapi/upload.py
@@ -1,26 +1,33 @@
+import io
 import json
 import os
+from typing import Union
 
 from canvasapi.util import combine_kwargs
 
+FileOrPathLike = Union[os.PathLike, str, io.IOBase, io.FileIO]
+"""
+A path or file-like object. May be either a :class:`os.PathLike`, a `str`, or a file-like object
+"""
+
 
 class Uploader(object):
     """
     Upload a file to Canvas.
     """
 
-    def __init__(self, requester, url, file, **kwargs):
+    def __init__(self, requester, url, file: FileOrPathLike, **kwargs):
         """
         :param requester: The :class:`canvasapi.requester.Requester` to pass requests through.
         :type requester: :class:`canvasapi.requester.Requester`
         :param url: The URL to upload the file to.
         :type url: str
         :param file: A file handler or path of the file to upload.
-        :type file: file or str
+        :type file: :class:`os.PathLike` or str
         """
-        if isinstance(file, str):
+        if isinstance(file, (os.PathLike, str)):
             if not os.path.exists(file):
-                raise IOError("File " + file + " does not exist.")
+                raise IOError("File {} does not exist.".format(os.fspath(file)))
             self._using_filename = True
         else:
             self._using_filename = False
diff --git a/canvasapi/user.py b/canvasapi/user.py
index 0cfb8c7..74c5079 100644
--- a/canvasapi/user.py
+++ b/canvasapi/user.py
@@ -6,7 +6,7 @@ from canvasapi.folder import Folder
 from canvasapi.license import License
 from canvasapi.paginated_list import PaginatedList
 from canvasapi.pairing_code import PairingCode
-from canvasapi.upload import Uploader
+from canvasapi.upload import FileOrPathLike, Uploader
 from canvasapi.usage_rights import UsageRights
 from canvasapi.util import combine_kwargs, obj_or_id, obj_or_str
 
@@ -954,7 +954,7 @@ class User(CanvasObject):
         )
         return response.json()
 
-    def upload(self, file, **kwargs):
+    def upload(self, file: FileOrPathLike, **kwargs):
         """
         Upload a file for a user.
 
diff --git a/setup.py b/setup.py
index ed7f9f4..212542b 100644
--- a/setup.py
+++ b/setup.py
@@ -1,5 +1,6 @@
 import re
 from os import path
+
 from setuptools import setup
 
 # get version number


Test patch:
diff --git a/tests/test_assignment.py b/tests/test_assignment.py
index e77a325..165970c 100644
--- a/tests/test_assignment.py
+++ b/tests/test_assignment.py
@@ -1,5 +1,6 @@
 import unittest
 import uuid
+from pathlib import Path
 
 import requests_mock
 
@@ -215,6 +216,24 @@ class TestAssignment(unittest.TestCase):
         finally:
             cleanup_file(filename)
 
+    def test_submit_file_pathlib(self, m):
+        register_uris({"assignment": ["submit", "upload", "upload_final"]}, m)
+
+        filename = Path("testfile_assignment_{}".format(uuid.uuid4().hex))
+        filename.write_bytes(b"test data")
+
+        try:
+            sub_type = "online_upload"
+            sub_dict = {"submission_type": sub_type}
+            submission = self.assignment.submit(sub_dict, filename)
+
+            self.assertIsInstance(submission, Submission)
+            self.assertTrue(hasattr(submission, "submission_type"))
+            self.assertEqual(submission.submission_type, sub_type)
+
+        finally:
+            cleanup_file(filename)
+
     def test_submit_file_wrong_type(self, m):
         filename = "testfile_assignment_{}".format(uuid.uuid4().hex)
         sub_type = "online_text_entry"
diff --git a/tests/test_uploader.py b/tests/test_uploader.py
index 73a234d..d19d049 100644
--- a/tests/test_uploader.py
+++ b/tests/test_uploader.py
@@ -1,5 +1,6 @@
 import unittest
 import uuid
+from pathlib import Path
 
 import requests_mock
 
@@ -34,6 +35,17 @@ class TestUploader(unittest.TestCase):
         self.assertIsInstance(result[1], dict)
         self.assertIn("url", result[1])
 
+    def test_start_pathlib(self, m):
+        requires = {"uploader": ["upload_response", "upload_response_upload_url"]}
+        register_uris(requires, m)
+
+        uploader = Uploader(self.requester, "upload_response", Path(self.filename))
+        result = uploader.start()
+
+        self.assertTrue(result[0])
+        self.assertIsInstance(result[1], dict)
+        self.assertIn("url", result[1])
+
     def test_start_path(self, m):
         requires = {"uploader": ["upload_response", "upload_response_upload_url"]}
         register_uris(requires, m)