Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign upRemove duplicate worker code #19
Conversation
c-w
added some commits
Aug 6, 2018
michaelperel
reviewed
Aug 6, 2018
| @@ -91,7 +95,8 @@ def _get_img_ids_and_features(self): | |||
| known_features.append(current_features) | |||
| return img_ids, np.array(known_features) | |||
|
|
|||
| def _prepare_matches(self, matches, that_img_id, distance_score): | |||
| @classmethod | |||
This comment has been minimized.
This comment has been minimized.
michaelperel
Aug 6, 2018
Collaborator
Should this be @staticmethod, as it does not seem that cls is used?
| def _delete_img(self, img_id): | ||
| self.logger.debug('deleting img') | ||
| for extension in self.allowed_file_extensions: | ||
| img_name = "{}.{}".format(img_id, extension) | ||
| fpath = os.path.join(self.img_dir, img_name) | ||
| try: | ||
| os.remove(fpath) | ||
| self.logger.debug("removed {}".format(img_id)) | ||
| except: | ||
| self.logger.debug("removed %s", img_id) |
This comment has been minimized.
This comment has been minimized.
michaelperel
Aug 6, 2018
Collaborator
Is it preferable to use printf style strings over newer format strings? (should we consider using python 3.6, perhaps and using f'{}' instead?)
This comment has been minimized.
This comment has been minimized.
c-w
Aug 6, 2018
Author
Member
Using %s here is by design since this is a logger call. If you do self.logger.debug('removed {}'.format(img_id)), the format string will always be allocated even if the log level is higher than debug. With self.logger.debug('removed %s', img_id), the logging framework will only format the string if it's actually going to be printed.
| @@ -1,3 +1,5 @@ | |||
| # pylint: disable=too-few-public-methods | |||
This comment has been minimized.
This comment has been minimized.
michaelperel
Aug 6, 2018
Collaborator
Should we specify the inline pylint instructions into a pylintrc or are they specific to these few cases?
This comment has been minimized.
This comment has been minimized.
c-w
Aug 6, 2018
Author
Member
These are just a few cases (some of which I'll also refactor). In general you want .pylintrc to be as empty as possible so that you get the full linting goodness.
c-w commentedAug 6, 2018
•
edited
See #9