From 79d0cbb181cc6b6f2777e678b4e9622780945d86 Mon Sep 17 00:00:00 2001 From: Kyle Date: Tue, 30 Jul 2019 10:30:40 +0900 Subject: [PATCH 1/3] Add coding conventions --- CONTRIBUTING.md | 76 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cd9ccbe96..872e4460c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -366,3 +366,79 @@ duration = float_or_none(video.get('durationMs'), scale=1000) view_count = int_or_none(video.get('views')) ``` +### Inline values + + Always inline values. + +#### Example + +A good rule of thumb is not to create variables that will only be used once. + +Correct: + +```python +return self.playlist_result( + [self._parse_brightcove_metadata(vid, vid.get('id'), headers) + for vid in json_data.get('videos', []) if vid.get('id')], + json_data.get('id'), json_data.get('name'), + json_data.get('description')) +``` + +Incorrect: + +```python +playlist_vids = json_data.get('videos', []) +playlist_id = json_data.get('id') +playlist_title = json_data.get('name') +playlist_description = json_data.get('description') + +return self.playlist_result( + [self._parse_brightcove_metadata(vid, vid.get('id'), headers) + for vid in playlist_vids if vid.get('id')], + playlist_id, playlist_title, playlist_description) +``` + +### Collapse fallbacks + +Multiple fallback values can quickly become unwieldy. Collapse multiple fallback values into a single expression via a list of meta values. + +#### Example + +Good: + +```python +description = self._html_search_meta( + ['og:description', 'description', 'twitter:description'], + webpage, 'description', default=None) +``` + +Unwieldy: + +```python +description = ( + self._og_search_description(webpage, default=None) + or self._html_search_meta('description', webpage, default=None) + or self._html_search_meta('twitter:description', webpage, default=None)) +``` + +### Trailing parentheses + +Always move trailing parentheses after the last argument. + +#### Example + +Correct: + +```python + lambda x: x['ResultSet']['Result'][0]['VideoUrlSet']['VideoUrl'], + list) +``` + +Incorrect: + +```python + lambda x: x['ResultSet']['Result'][0]['VideoUrlSet']['VideoUrl'], + list, +) +``` + From 2590fb724fe26d262cf99a3370297f4cb68a55ca Mon Sep 17 00:00:00 2001 From: Kyle Date: Wed, 31 Jul 2019 09:03:51 +0900 Subject: [PATCH 2/3] Requested changes --- CONTRIBUTING.md | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 872e4460c..1813f5473 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -368,18 +368,16 @@ view_count = int_or_none(video.get('views')) ### Inline values - Always inline values. +Extracting variables is acceptable for reducing code duplication and improving readability of complex expressions. However, you should avoid extracting variables used only once and moving them to opposite parts of the extractor file, which makes reading the linear flow difficult. #### Example -A good rule of thumb is not to create variables that will only be used once. - Correct: ```python return self.playlist_result( [self._parse_brightcove_metadata(vid, vid.get('id'), headers) - for vid in json_data.get('videos', []) if vid.get('id')], + for vid in json_data.get('videos', []) if vid.get('id')], json_data.get('id'), json_data.get('name'), json_data.get('description')) ``` @@ -387,15 +385,12 @@ return self.playlist_result( Incorrect: ```python -playlist_vids = json_data.get('videos', []) -playlist_id = json_data.get('id') -playlist_title = json_data.get('name') -playlist_description = json_data.get('description') +id = json_data.get('id') return self.playlist_result( [self._parse_brightcove_metadata(vid, vid.get('id'), headers) - for vid in playlist_vids if vid.get('id')], - playlist_id, playlist_title, playlist_description) + for vid in json_data.get('videos', []) if vid.get('id')], + id, json_data.get('name'), json_data.get('description')) ``` ### Collapse fallbacks From 9ecf3eee2da8ed0acb90ad2107d42f6b1eecb01e Mon Sep 17 00:00:00 2001 From: Kyle Date: Thu, 1 Aug 2019 10:19:49 +0900 Subject: [PATCH 3/3] Simplify example --- CONTRIBUTING.md | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1813f5473..d0e0a5637 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -375,22 +375,15 @@ Extracting variables is acceptable for reducing code duplication and improving r Correct: ```python -return self.playlist_result( - [self._parse_brightcove_metadata(vid, vid.get('id'), headers) - for vid in json_data.get('videos', []) if vid.get('id')], - json_data.get('id'), json_data.get('name'), - json_data.get('description')) +title = self._html_search_regex(r'([^<]+)', webpage, 'title') ``` Incorrect: ```python -id = json_data.get('id') - -return self.playlist_result( - [self._parse_brightcove_metadata(vid, vid.get('id'), headers) - for vid in json_data.get('videos', []) if vid.get('id')], - id, json_data.get('name'), json_data.get('description')) +TITLE_RE = r'([^<]+)' +# ...some lines of code... +title = self._html_search_regex(TITLE_RE, webpage, 'title') ``` ### Collapse fallbacks