From e6055d3a8e695e3228083a851df6c9c6cc0f1817 Mon Sep 17 00:00:00 2001 From: Talha Date: Sat, 25 Nov 2023 21:33:32 +0500 Subject: [PATCH] refactor: removed pylint warnings --- openedx_cmi5_xblock/__init__.py | 4 +- openedx_cmi5_xblock/openedx_cmi5_xblock.py | 168 ++++++++------------- tox.ini | 2 +- 3 files changed, 65 insertions(+), 109 deletions(-) diff --git a/openedx_cmi5_xblock/__init__.py b/openedx_cmi5_xblock/__init__.py index b8d33dc..6ea5c3a 100644 --- a/openedx_cmi5_xblock/__init__.py +++ b/openedx_cmi5_xblock/__init__.py @@ -1,6 +1,4 @@ -""" -Init for the CMI5XBlock package. -""" +"""Init for the CMI5XBlock package.""" from .openedx_cmi5_xblock import CMI5XBlock diff --git a/openedx_cmi5_xblock/openedx_cmi5_xblock.py b/openedx_cmi5_xblock/openedx_cmi5_xblock.py index 0bc3dd3..f5228f6 100644 --- a/openedx_cmi5_xblock/openedx_cmi5_xblock.py +++ b/openedx_cmi5_xblock/openedx_cmi5_xblock.py @@ -1,4 +1,4 @@ -"""Openedx CMI5 XBlock implementation """ +"""Openedx CMI5 XBlock implementation.""" import hashlib import json @@ -39,9 +39,8 @@ def _(text): @XBlock.wants('settings') @XBlock.wants('enrollments') class CMI5XBlock(XBlock, CompletableXBlockMixin): - """ - This is the main xblock class with all the members defined - """ + """The main xblock class with all the members defined.""" + display_name = String( display_name=_('Display Name'), help=_('Display name'), @@ -109,6 +108,7 @@ class CMI5XBlock(XBlock, CompletableXBlockMixin): has_author_view = True def render_template(self, template_path, context): + """Render a template with the provided context.""" template_str = self.resource_string(template_path) template = Template(template_str) return template.render(Context(context)) @@ -121,15 +121,19 @@ def resource_string(path): @staticmethod def json_response(data): + """Generate a JSON response.""" return Response(json.dumps(data), content_type='application/json', charset='utf8') def get_current_user_attr(self, attr: str): + """Get the value of a specific attribute for the current user.""" return self.get_current_user().opt_attrs.get(attr) def get_current_user(self): + """Get the current user.""" return self.runtime.service(self, 'user').get_current_user() def author_view(self, context=None): + """View for authors to preview content.""" context = context or {} if not self.index_page_path: context['message'] = 'Click "Edit" to modify this module and upload a new CMI5 package.' @@ -137,8 +141,10 @@ def author_view(self, context=None): def studio_view(self, context=None): """ - This method generates the studio view, including the display of various fields - related to the CMI5 XBlock, and initializes the required CSS and JavaScript. + It generates the studio view. + + Including the display of various fields related to the CMI5 XBlock, + and initializes the required CSS and JavaScript. """ studio_context = { 'field_display_name': self.fields['display_name'], @@ -158,10 +164,7 @@ def studio_view(self, context=None): return frag def student_view(self, context=None): - """ - The primary view of the CMI5XBlock, shown to students when viewing courses. - """ - + """The primary view of the CMI5XBlock, shown to students when viewing courses.""" student_context = { 'title': self.display_name, 'index_page_url': self.index_page_url, @@ -185,6 +188,7 @@ def student_view(self, context=None): def index_page_url(self): """ Gets the URL of the CMI5 index page. + Returns an empty string if the package metadata or index page path is not available. """ if not self.package_meta or not self.index_page_path: @@ -207,22 +211,19 @@ def index_page_url(self): @property def extract_folder_path(self): """ - This path needs to depend on the content of the cmi5 package. Otherwise, served media files might become - stale when the package is update. + It needs to depend on the content of the cmi5 package. + + Otherwise, served media files might become stale when the package is update. """ return os.path.join(self.extract_folder_base_path, self.package_meta['sha1']) @property def extract_folder_base_path(self): - """ - Path to the folder where packages will be extracted. - """ + """Path to the folder where packages will be extracted.""" return os.path.join(self.cmi5_location(), self.location.block_id) def is_url(self, path): - """ - Checks if the given path is a valid URL. - """ + """Checks if the given path is a valid URL.""" try: validator = URLValidator(verify_exists=False) validator(path) @@ -231,15 +232,11 @@ def is_url(self, path): return True def is_params_exist(self, url): - """ - Checks if query parameters exist in the given URL. - """ + """Checks if query parameters exist in the given URL.""" return '?' in url def get_launch_url_params(self): - """ - Constructs and returns launch URL parameters for CMI5 integration - """ + """Constructs and returns launch URL parameters for CMI5 integration.""" parameters = { 'fetch': urllib.parse.quote_plus(self.runtime.handler_url(self, 'lrs_auth_endpoint', thirdparty=True)), 'endpoint': urllib.parse.quote_plus( @@ -266,9 +263,7 @@ def get_launch_url_params(self): @XBlock.handler def studio_submit(self, request, _suffix): - """ - Handles the submission of the CMI5 XBlock studio form. - """ + """Handles the submission of the CMI5 XBlock studio form.""" self.display_name = request.params['display_name'] self.width = parse_int(request.params['width'], None) self.height = parse_int(request.params['height'], None) @@ -296,9 +291,7 @@ def studio_submit(self, request, _suffix): return self.json_response(response) def update_package_meta(self, package_file): - """ - Updates the package metadata based on the provided package file. - """ + """Updates the package metadata based on the provided package file.""" self.package_meta['sha1'] = self.get_sha1(package_file) self.package_meta['name'] = package_file.name self.package_meta['last_updated'] = timezone.now().strftime(DateTime.DATETIME_FORMAT) @@ -306,9 +299,7 @@ def update_package_meta(self, package_file): package_file.seek(0) def clean_storage(self): - """ - Cleans the storage by removing the previously unzipped content. - """ + """Cleans the storage by removing the previously unzipped content.""" if self.storage.exists(self.extract_folder_base_path): logger.info('Removing previously unzipped "%s"', self.extract_folder_base_path) self.recursive_delete(self.extract_folder_base_path) @@ -316,6 +307,7 @@ def clean_storage(self): def recursive_delete(self, root): """ Recursively delete the contents of a directory in the Django default storage. + Unfortunately, this will not delete empty folders, as the default FileSystemStorage implementation does not allow it. """ @@ -326,9 +318,7 @@ def recursive_delete(self, root): self.storage.delete(os.path.join(root, f)) def extract_package(self, package_file): - """ - Extracts content from the provided CMI5 package file. - """ + """Extracts content from the provided CMI5 package file.""" ext = package_file.name.split('.')[-1].lower() if ext == 'zip': self.extract_zip_file(package_file) @@ -338,9 +328,7 @@ def extract_package(self, package_file): raise CMI5Error(f'Could not support {ext} file') def extract_zip_file(self, package_file): - """ - Extracts content from a zip file within the CMI5 package. - """ + """Extracts content from a zip file within the CMI5 package.""" with zipfile.ZipFile(package_file, 'r') as cmi5_zipfile: zipinfos = cmi5_zipfile.infolist() root_path = None @@ -364,17 +352,12 @@ def extract_zip_file(self, package_file): self.storage.save(dest_path, ContentFile(cmi5_zipfile.read(zipinfo.filename))) def save_xml_file(self, package_file): - """ - Saves an XML file from the CMI5 package. - """ + """Saves an XML file from the CMI5 package.""" dest_path = os.path.join(self.extract_folder_path, package_file.filename) self.storage.save(dest_path, ContentFile(package_file.filename)) def update_package_fields(self): - """ - Update version and index page path fields. - """ - + """Update version and index page path fields.""" cmi5_path = self.find_file_path(CMI5XML_FILENAME) cmi5_file = self.storage.open(cmi5_path) tree = ET.parse(cmi5_file) @@ -396,9 +379,7 @@ def update_package_fields(self): self.index_page_path = self.find_relative_file_path('index.html') def set_course_detail(self, prefix, root): - """ - Extracts course details from the provided XML root. - """ + """Extracts course details from the provided XML root.""" course_data = {} try: @@ -417,10 +398,7 @@ def set_course_detail(self, prefix, root): @XBlock.handler def lrs_endpoint(self, request, _suffix): - """ - Handles requests related to the Learning Record Store (LRS) endpoint. - """ - + """Handles requests related to the Learning Record Store (LRS) endpoint.""" if request.params.get('statementId') and request.method == 'PUT': statement_data = self.get_request_body(request) @@ -452,44 +430,32 @@ def lrs_endpoint(self, request, _suffix): return self.json_response({'success': True}) def get_request_body(self, request): - """ - Gets the JSON body from an HTTP request. - """ + """Gets the JSON body from an HTTP request.""" return json.loads(request.body.decode('utf-8')) def publish_grade(self): - """ - Publishes the grade to the XBlock runtime. - """ + """Publishes the grade to the XBlock runtime.""" self.runtime.publish(self, 'grade', {'value': self.get_grade(), 'max_value': self.weight}) def get_grade(self): - """ - Calculates and returns the normalized grade. - """ + """Calculates and returns the normalized grade.""" lesson_score = 0 if self.is_failed else self.lesson_score return lesson_score * self.weight @property def is_failed(self): - """ - Checks if the lesson is in a failed status. - """ + """Checks if the lesson is in a failed status.""" return self.lesson_status == 'failed' def is_cmi5_object(self, categories): - """ - Checks if the given categories include the cmi5 category. - """ + """Checks if the given categories include the cmi5 category.""" if categories is None: return False cmi5_category = 'https://w3id.org/xapi/cmi5/context/categories/cmi5' return any([category['id'] == cmi5_category for category in categories]) def get_launch_state_data(self): - """ - Generates and returns launch state data for the XBlock. - """ + """Generates and returns launch state data for the XBlock.""" return { 'contextTemplate': { 'registration': self.get_enrollment_uuid(), @@ -515,9 +481,7 @@ def get_launch_state_data(self): @XBlock.handler def lrs_auth_endpoint(self, request, _suffix): - """ - Handles requests to the LRS authentication endpoint. - """ + """Handles requests to the LRS authentication endpoint.""" user_id = self.get_current_user_attr('edx-platform.user_id') session_id = request.cookies.get('sessionid', 'auth-session-id') authtoken = 'user-id:{0}_session-id:{1}'.format(user_id, session_id) @@ -526,9 +490,7 @@ def lrs_auth_endpoint(self, request, _suffix): return self.json_response({'auth-token': authtoken}) def get_erollment_id(self): - """ - Retrieves the enrollment ID of the current user for the XBlock's course. - """ + """Retrieves the enrollment ID of the current user for the XBlock's course.""" user_id = self.get_current_user_attr('edx-platform.user_id') course_id = self.runtime.course_id try: @@ -540,23 +502,21 @@ def get_erollment_id(self): return 'anonymous' def get_enrollment_uuid(self): - """ - Generates and returns the enrollment UUID based on the enrollment ID. - """ + """Generates and returns the enrollment UUID based on the enrollment ID.""" base_id = uuid.UUID('2af01743-8d97-423e-988a-25c69fa4ea66') enrollment_uuid = uuid.uuid5(base_id, 'openedx-enrollment-id:{0}'.format(self.get_erollment_id())) return str(enrollment_uuid) def find_relative_file_path(self, filename): - """ - Finds the relative file path within the XBlock's extract folder. - """ + """Finds the relative file path within the XBlock's extract folder.""" return os.path.relpath(self.find_file_path(filename), self.extract_folder_path) def find_file_path(self, filename): """ - Search recursively in the extracted folder for a given file. Path of the first - found file will be returned. Raise a CMI5Error if file cannot be found. + Search recursively in the extracted folder for a given file. + + Path of the first found file will be returned. + Raise a CMI5Error if file cannot be found. """ path = self.get_file_path(filename, self.extract_folder_path) if path is None: @@ -564,9 +524,7 @@ def find_file_path(self, filename): return path def get_file_path(self, filename, root): - """ - Same as `find_file_path`, but don't raise error on file not found. - """ + """Same as `find_file_path`, but don't raise error on file not found.""" subfolders, files = self.storage.listdir(root) for f in files: if f == filename: @@ -579,17 +537,16 @@ def get_file_path(self, filename, root): def cmi5_location(self): """ - Unzipped files will be stored in a media folder with this name, and thus - accessible at a url with that also includes this name. + Unzipped files will be stored in a media folder with this name. + + thus accessible at a url with that also includes this name. """ default_cmi5_location = 'cmi5' return self.xblock_settings.get('LOCATION', default_cmi5_location) @staticmethod def get_sha1(file_descriptor): - """ - Get file hex digest (fingerprint). - """ + """Get file hex digest (fingerprint).""" block_size = 8 * 1024 sha1 = hashlib.sha1() while True: @@ -603,7 +560,9 @@ def get_sha1(file_descriptor): @property def storage(self): """ - Return the storage backend used to store the assets of this xblock. This is a cached property. + Return the storage backend used to store the assets of this xblock. + + This is a cached property. """ if not getattr(self, '_storage', None): def get_default_storage(_xblock): @@ -618,9 +577,7 @@ def get_default_storage(_xblock): @property def xblock_settings(self): - """ - Return a dict of settings associated to this XBlock. - """ + """Return a dict of settings associated to this XBlock.""" settings_service = self.runtime.service(self, 'settings') or {} if not settings_service: return {} @@ -645,7 +602,9 @@ def workbench_scenarios(): def parse_int(value, default): """ - Parses an integer, returning the parsed value or a default if unsuccessful. + Parses an integer. + + returning the parsed value or a default if unsuccessful. """ try: return int(value) @@ -655,7 +614,9 @@ def parse_int(value, default): def parse_float(value, default): """ - Parses a float, returning the parsed value or a default if unsuccessful. + Parses a float. + + Returning the parsed value or a default if unsuccessful. """ try: return float(value) @@ -664,9 +625,7 @@ def parse_float(value, default): def parse_validate_positive_float(value, name): - """ - Parse and validate a given value as a positive float. - """ + """Parse and validate a given value as a positive float.""" try: parsed = float(value) except (TypeError, ValueError): @@ -677,7 +636,6 @@ def parse_validate_positive_float(value, name): class CMI5Error(Exception): - """ - Base exception class for CMI5-related errors. - """ + """Base exception class for CMI5-related errors.""" + pass diff --git a/tox.ini b/tox.ini index 39de4ac..5c6b699 100644 --- a/tox.ini +++ b/tox.ini @@ -77,7 +77,7 @@ deps = commands = pylint openedx_cmi5_xblock test_utils manage.py --disable=E1136,E0401,E1101,W0718,W0612,R1729,W0201,W0707,W0107,E0013 pycodestyle openedx_cmi5_xblock manage.py - pydocstyle openedx_cmi5_xblock manage.py + pydocstyle openedx_cmi5_xblock manage.py --ignore=D401,D203,D212 isort --check-only --diff test_utils openedx_cmi5_xblock manage.py [testenv:pii_check]