Open
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Comment on lines
-169
to
+174
| for root, _dirs, _files in os.walk(LIBRARY_DIR, topdown=False): | ||
| # setuptools is including __pycache__ for some reason (#1605) | ||
| if root.endswith('/__pycache__'): | ||
| remove_dirs.append(root) | ||
| remove_dirs.extend( | ||
| root | ||
| for root, _dirs, _files in os.walk(LIBRARY_DIR, topdown=False) | ||
| if root.endswith('/__pycache__') | ||
| ) | ||
|
|
Author
There was a problem hiding this comment.
Function main refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend)
This removes the following comments ( why? ):
# setuptools is including __pycache__ for some reason (#1605)
Comment on lines
-25
to
+27
| node = nodes.reference(rawtext, utils.unescape(name), | ||
| refuri='{}?q={}'.format(base, name), | ||
| **options) | ||
| return node | ||
| return nodes.reference( | ||
| rawtext, utils.unescape(name), refuri=f'{base}?q={name}', **options | ||
| ) |
Author
There was a problem hiding this comment.
Function make_link_node refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable) - Replace call to format with f-string. (
use-fstring-for-formatting)
| for cls in (types.PeerUser, types.PeerChat, types.PeerChannel): | ||
| result = self.__dict__.get(utils.get_peer_id(cls(item))) | ||
| if result: | ||
| if result := self.__dict__.get(utils.get_peer_id(cls(item))): |
Author
There was a problem hiding this comment.
Function EntityCache.__getitem__ refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
| """Ensures that the parent directory exists""" | ||
| parent = os.path.dirname(file_path) | ||
| if parent: | ||
| if parent := os.path.dirname(file_path): |
Author
There was a problem hiding this comment.
Function ensure_parent_dir_exists refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
Comment on lines
-114
to
+113
| if force_retry and not (retries is None or retries < 0): | ||
| if force_retry and retries is not None and retries >= 0: |
Author
There was a problem hiding this comment.
Function retry_range refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan)
| return coro | ||
| else: | ||
| return loop.run_until_complete(coro) | ||
| return coro if loop.is_running() else loop.run_until_complete(coro) |
Author
There was a problem hiding this comment.
Function _syncify_wrap.syncified refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp)
| if ( | ||
| not name.startswith('_') or name == '__call__' | ||
| ) and inspect.iscoroutinefunction(getattr(t, name)): | ||
| _syncify_wrap(t, name) |
Author
There was a problem hiding this comment.
Function syncify refactored with the following changes:
- Merge nested if conditions (
merge-nested-ifs)
| if isinstance(entity, types.User): | ||
| if entity.last_name and entity.first_name: | ||
| return '{} {}'.format(entity.first_name, entity.last_name) | ||
| return f'{entity.first_name} {entity.last_name}' |
Author
There was a problem hiding this comment.
Function get_display_name refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-138
to
+140
| raise TypeError('Cannot cast {} to any kind of {}.'.format( | ||
| type(entity).__name__, target)) | ||
| raise TypeError( | ||
| f'Cannot cast {type(entity).__name__} to any kind of {target}.' | ||
| ) |
Author
There was a problem hiding this comment.
Function _raise_cast_fail refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-479
to
+490
| else: | ||
| attrs, mime = get_attributes( | ||
| media, | ||
| attributes=attributes, | ||
| force_document=force_document, | ||
| voice_note=voice_note, | ||
| video_note=video_note, | ||
| supports_streaming=supports_streaming | ||
| ) | ||
| return types.InputMediaUploadedDocument( | ||
| file=media, mime_type=mime, attributes=attrs, force_file=force_document, | ||
| ttl_seconds=ttl) | ||
| attrs, mime = get_attributes( | ||
| media, | ||
| attributes=attributes, | ||
| force_document=force_document, | ||
| voice_note=voice_note, | ||
| video_note=video_note, | ||
| supports_streaming=supports_streaming | ||
| ) | ||
| return types.InputMediaUploadedDocument( | ||
| file=media, mime_type=mime, attributes=attrs, force_file=force_document, | ||
| ttl_seconds=ttl) |
Author
There was a problem hiding this comment.
Function get_input_media refactored with the following changes:
- Remove unnecessary else after guard condition (
remove-unnecessary-else)
| pass | ||
|
|
||
| raise TypeError('Invalid message type: {}'.format(type(message))) | ||
| raise TypeError(f'Invalid message type: {type(message)}') |
Author
There was a problem hiding this comment.
Function get_message_id refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-638
to
+650
| if getattr(file, 'seekable', None): | ||
| seekable = file.seekable() | ||
| else: | ||
| seekable = False | ||
|
|
||
| seekable = file.seekable() if getattr(file, 'seekable', None) else False | ||
| if not seekable: | ||
| return None | ||
|
|
||
| pos = stream.tell() | ||
| filename = getattr(file, 'name', '') | ||
|
|
||
| parser = hachoir.parser.guess.guessParser(hachoir.stream.InputIOStream( | ||
| stream, | ||
| source='file:' + filename, | ||
| tags=[], | ||
| filename=filename | ||
| )) | ||
| parser = hachoir.parser.guess.guessParser( | ||
| hachoir.stream.InputIOStream( | ||
| stream, source=f'file:{filename}', tags=[], filename=filename | ||
| ) | ||
| ) | ||
|
|
Author
There was a problem hiding this comment.
Function _get_metadata refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp) - Use f-string instead of string concatenation (
use-fstring-for-concatenation) - Lift repeated conditional into its own if statement (
lift-duplicated-conditional) - Swap positions of nested conditionals (
swap-nested-ifs)
Comment on lines
-684
to
+682
| m = _get_metadata(file) | ||
| if m: | ||
| if m := _get_metadata(file): |
Author
There was a problem hiding this comment.
Function get_attributes refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp) - Move setting of default value for variable into
elsebranch (introduce-default-else) - Use named expression to simplify assignment and conditional (
use-named-expression) - Move assignment closer to its usage within a block (
move-assign-in-block)
Comment on lines
-843
to
+837
| return ('.' + kind) if kind else '' | ||
| return f'.{kind}' if kind else '' | ||
| elif isinstance(file, io.IOBase) and not isinstance(file, io.TextIOBase) and file.seekable(): | ||
| kind = imghdr.what(file) | ||
| return ('.' + kind) if kind is not None else '' | ||
| return f'.{kind}' if kind is not None else '' |
Author
There was a problem hiding this comment.
Function _get_extension refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation)
Comment on lines
-860
to
+851
| if match: | ||
| return True | ||
| else: | ||
| return isinstance(resolve_bot_file_id(file), types.Photo) | ||
| return True if match else isinstance(resolve_bot_file_id(file), types.Photo) |
Author
There was a problem hiding this comment.
Function is_image refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp)
Comment on lines
-828
to
+835
| raise ValueError( | ||
| 'No such action "{}"'.format(action)) from None | ||
| raise ValueError(f'No such action "{action}"') from None | ||
| elif not isinstance(action, types.TLObject) or action.SUBCLASS_OF_ID != 0x20b2cc21: | ||
| # 0x20b2cc21 = crc32(b'SendMessageAction') | ||
| if isinstance(action, type): | ||
| raise ValueError('You must pass an instance, not the class') | ||
| else: | ||
| raise ValueError('Cannot use {} as action'.format(action)) | ||
| raise ValueError(f'Cannot use {action} as action') |
Author
There was a problem hiding this comment.
Function ChatMethods.action refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-119
to
+120
| for entity in entities: | ||
| peers.append(types.InputDialogPeer( | ||
| await self.client.get_input_entity(entity))) | ||
|
|
||
| peers.extend(types.InputDialogPeer( | ||
| await self.client.get_input_entity(entity)) for entity in entities) |
Author
There was a problem hiding this comment.
Function _DraftsIter._init refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend)
Comment on lines
-313
to
+311
| if not entity or utils.is_list_like(entity): | ||
| return items | ||
| else: | ||
| return items[0] | ||
| return items if not entity or utils.is_list_like(entity) else items[0] |
Author
There was a problem hiding this comment.
Function DialogMethods.get_drafts refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp)
Comment on lines
-383
to
+382
| if not utils.is_list_like(entity): | ||
| entities = [await self.get_input_entity(entity)] | ||
| else: | ||
| entities = await asyncio.gather( | ||
| *(self.get_input_entity(x) for x in entity)) | ||
| entities = ( | ||
| await asyncio.gather(*(self.get_input_entity(x) for x in entity)) | ||
| if utils.is_list_like(entity) | ||
| else [await self.get_input_entity(entity)] | ||
| ) |
Author
There was a problem hiding this comment.
Function DialogMethods.edit_folder refactored with the following changes:
- Swap if/else branches of if expression to remove negation (
swap-if-expression) - Replace if statement with if expression (
assign-if-exp)
Comment on lines
-446
to
+441
| if isinstance(entity, types.Chat): | ||
| deactivated = entity.deactivated | ||
| else: | ||
| deactivated = False | ||
|
|
||
| deactivated = entity.deactivated if isinstance(entity, types.Chat) else False |
Author
There was a problem hiding this comment.
Function DialogMethods.delete_dialog refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp)
Comment on lines
-259
to
+282
| if not hasattr(entity, 'chat_photo'): | ||
| return None | ||
|
|
||
| return await self._download_photo( | ||
| entity.chat_photo, file, date=None, | ||
| thumb=thumb, progress_callback=None | ||
| ) | ||
|
|
||
| for attr in ('username', 'first_name', 'title'): | ||
| possible_names.append(getattr(entity, attr, None)) | ||
| ) if hasattr(entity, 'chat_photo') else None | ||
| possible_names.extend( | ||
| getattr(entity, attr, None) | ||
| for attr in ('username', 'first_name', 'title') | ||
| ) | ||
|
|
||
| photo = entity.photo | ||
|
|
||
| if isinstance(photo, (types.UserProfilePhoto, types.ChatPhoto)): | ||
| dc_id = photo.dc_id | ||
| loc = types.InputPeerPhotoFileLocation( | ||
| peer=await self.get_input_entity(entity), | ||
| photo_id=photo.photo_id, | ||
| big=download_big | ||
| ) | ||
| else: | ||
| if not isinstance(photo, (types.UserProfilePhoto, types.ChatPhoto)): | ||
| # It doesn't make any sense to check if `photo` can be used | ||
| # as input location, because then this method would be able | ||
| # to "download the profile photo of a message", i.e. its | ||
| # media which should be done with `download_media` instead. | ||
| return None | ||
|
|
||
| dc_id = photo.dc_id | ||
| loc = types.InputPeerPhotoFileLocation( | ||
| peer=await self.get_input_entity(entity), | ||
| photo_id=photo.photo_id, | ||
| big=download_big | ||
| ) |
Author
There was a problem hiding this comment.
Function DownloadMethods.download_profile_photo refactored with the following changes:
- Swap if/else branches of if expression to remove negation (
swap-if-expression) - Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp) - Replace a for append loop with list extend (
for-append-to-extend) - Swap if/else branches (
swap-if-else-branches) - Remove unnecessary else after guard condition (
remove-unnecessary-else)
Comment on lines
-401
to
+405
| if isinstance(media, types.MessageService): | ||
| if isinstance(message.action, | ||
| types.MessageActionChatEditPhoto): | ||
| media = media.photo | ||
|
|
||
| if isinstance(media, types.MessageMediaWebPage): | ||
| if isinstance(media.webpage, types.WebPage): | ||
| media = media.webpage.document or media.webpage.photo | ||
| if isinstance(media, types.MessageService) and isinstance( | ||
| message.action, types.MessageActionChatEditPhoto | ||
| ): | ||
| media = media.photo | ||
|
|
||
| if isinstance(media, types.MessageMediaWebPage) and isinstance( | ||
| media.webpage, types.WebPage | ||
| ): | ||
| media = media.webpage.document or media.webpage.photo |
Author
There was a problem hiding this comment.
Function DownloadMethods.download_media refactored with the following changes:
- Merge nested if conditions (
merge-nested-ifs)
Comment on lines
-514
to
+511
| if not file_size: | ||
| part_size_kb = 64 # Reasonable default | ||
| else: | ||
| part_size_kb = utils.get_appropriated_part_size(file_size) | ||
|
|
||
| part_size_kb = utils.get_appropriated_part_size(file_size) if file_size else 64 |
Author
There was a problem hiding this comment.
Function DownloadMethods._download_file refactored with the following changes:
- Swap if/else branches of if expression to remove negation (
swap-if-expression) - Swap positions of nested conditionals (
swap-nested-ifs) - Hoist repeated code outside conditional statement (
hoist-statement-from-if) - Replace if statement with if expression (
assign-if-exp)
This removes the following comments ( why? ):
# Reasonable default
Comment on lines
-849
to
+842
| possible_names.append('{} - {}'.format( | ||
| attr.performer, attr.title | ||
| )) | ||
| possible_names.append(f'{attr.performer} - {attr.title}') |
Author
There was a problem hiding this comment.
Function DownloadMethods._get_kind_and_names refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-1038
to
+1029
| result = os.path.join(directory, '{} ({}){}'.format(name, i, ext)) | ||
| result = os.path.join(directory, f'{name} ({i}){ext}') |
Author
There was a problem hiding this comment.
Function DownloadMethods._get_proper_filename refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-811
to
-831
| session = self._exported_sessions.get(cdn_redirect.dc_id) | ||
| if not session: | ||
| dc = await self._get_dc(cdn_redirect.dc_id, cdn=True) | ||
| session = self.session.clone() | ||
| await session.set_dc(dc.id, dc.ip_address, dc.port) | ||
| self._exported_sessions[cdn_redirect.dc_id] = session | ||
|
|
||
| self._log[__name__].info('Creating new CDN client') | ||
| client = TelegramBaseClient( | ||
| session, self.api_id, self.api_hash, | ||
| proxy=self._sender.connection.conn.proxy, | ||
| timeout=self._sender.connection.get_timeout() | ||
| ) | ||
|
|
||
| # This will make use of the new RSA keys for this specific CDN. | ||
| # | ||
| # We won't be calling GetConfigRequest because it's only called | ||
| # when needed by ._get_dc, and also it's static so it's likely | ||
| # set already. Avoid invoking non-CDN methods by not syncing updates. | ||
| client.connect(_sync_updates=False) | ||
| return client |
Author
There was a problem hiding this comment.
Function TelegramBaseClient._get_cdn_client refactored with the following changes:
- Remove unreachable code (
remove-unreachable-code)
This removes the following comments ( why? ):
# when needed by ._get_dc, and also it's static so it's likely
# This will make use of the new RSA keys for this specific CDN.
# set already. Avoid invoking non-CDN methods by not syncing updates.
#
# We won't be calling GetConfigRequest because it's only called
Comment on lines
-400
to
+418
| if not self._entity_cache.ensure_cached(update): | ||
| # We could add a lock to not fetch the same pts twice if we are | ||
| # already fetching it. However this does not happen in practice, | ||
| # which makes sense, because different updates have different pts. | ||
| if self._state_cache.update(update, check_only=True): | ||
| # If the update doesn't have pts, fetching won't do anything. | ||
| # For example, UpdateUserStatus or UpdateChatUserTyping. | ||
| try: | ||
| await self._get_difference(update, channel_id, pts_date) | ||
| except OSError: | ||
| pass # We were disconnected, that's okay | ||
| except errors.RPCError: | ||
| # There's a high chance the request fails because we lack | ||
| # the channel. Because these "happen sporadically" (#1428) | ||
| # we should be okay (no flood waits) even if more occur. | ||
| pass | ||
| except ValueError: | ||
| # There is a chance that GetFullChannelRequest and GetDifferenceRequest | ||
| # inside the _get_difference() function will end up with | ||
| # ValueError("Request was unsuccessful N time(s)") for whatever reasons. | ||
| pass | ||
| if not self._entity_cache.ensure_cached( | ||
| update | ||
| ) and self._state_cache.update(update, check_only=True): | ||
| # If the update doesn't have pts, fetching won't do anything. | ||
| # For example, UpdateUserStatus or UpdateChatUserTyping. | ||
| try: | ||
| await self._get_difference(update, channel_id, pts_date) | ||
| except OSError: | ||
| pass # We were disconnected, that's okay | ||
| except errors.RPCError: | ||
| # There's a high chance the request fails because we lack | ||
| # the channel. Because these "happen sporadically" (#1428) | ||
| # we should be okay (no flood waits) even if more occur. | ||
| pass | ||
| except ValueError: | ||
| # There is a chance that GetFullChannelRequest and GetDifferenceRequest | ||
| # inside the _get_difference() function will end up with | ||
| # ValueError("Request was unsuccessful N time(s)") for whatever reasons. | ||
| pass |
Author
There was a problem hiding this comment.
Function UpdateMethods._dispatch_update refactored with the following changes:
- Merge nested if conditions (
merge-nested-ifs) - Use named expression to simplify assignment and conditional (
use-named-expression)
This removes the following comments ( why? ):
# already fetching it. However this does not happen in practice,
# We could add a lock to not fetch the same pts twice if we are
# which makes sense, because different updates have different pts.
Comment on lines
-541
to
+539
| assert isinstance(channel_id, int), 'channel_id was {}, not int in {}'.format(type(channel_id), update) | ||
| assert isinstance( | ||
| channel_id, int | ||
| ), f'channel_id was {type(channel_id)}, not int in {update}' | ||
|
|
Author
There was a problem hiding this comment.
Function UpdateMethods._get_difference refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-598
to
-628
| try: | ||
| self._log[__name__].info( | ||
| 'Asking for the current state after reconnect...') | ||
|
|
||
| # TODO consider: | ||
| # If there aren't many updates while the client is disconnected | ||
| # (I tried with up to 20), Telegram seems to send them without | ||
| # asking for them (via updates.getDifference). | ||
| # | ||
| # On disconnection, the library should probably set a "need | ||
| # difference" or "catching up" flag so that any new updates are | ||
| # ignored, and then the library should call updates.getDifference | ||
| # itself to fetch them. | ||
| # | ||
| # In any case (either there are too many updates and Telegram | ||
| # didn't send them, or there isn't a lot and Telegram sent them | ||
| # but we dropped them), we fetch the new difference to get all | ||
| # missed updates. I feel like this would be the best solution. | ||
|
|
||
| # If a disconnection occurs, the old known state will be | ||
| # the latest one we were aware of, so we can catch up since | ||
| # the most recent state we were aware of. | ||
| await self.catch_up() | ||
|
|
||
| self._log[__name__].info('Successfully fetched missed updates') | ||
| except errors.RPCError as e: | ||
| self._log[__name__].warning('Failed to get missed updates after ' | ||
| 'reconnect: %r', e) | ||
| except Exception: | ||
| self._log[__name__].exception( | ||
| 'Unhandled exception while getting update difference after reconnect') |
Author
There was a problem hiding this comment.
Function UpdateMethods._handle_auto_reconnect refactored with the following changes:
- Remove unreachable code (
remove-unreachable-code)
This removes the following comments ( why? ):
# didn't send them, or there isn't a lot and Telegram sent them
# itself to fetch them.
# but we dropped them), we fetch the new difference to get all
# ignored, and then the library should call updates.getDifference
# If a disconnection occurs, the old known state will be
# difference" or "catching up" flag so that any new updates are
# the most recent state we were aware of.
# If there aren't many updates while the client is disconnected
# the latest one we were aware of, so we can catch up since
# missed updates. I feel like this would be the best solution.
# In any case (either there are too many updates and Telegram
# On disconnection, the library should probably set a "need
# TODO consider:
#
# asking for them (via updates.getDifference).
# (I tried with up to 20), Telegram seems to send them without
| else: | ||
| captions = [caption] | ||
|
|
||
| captions = caption if utils.is_list_like(caption) else [caption] |
Author
There was a problem hiding this comment.
Function UploadMethods.sendfile refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp)
Author
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.11%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branch
backuprefactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
backupbranch, then run:Help us improve this pull request!