Support list of names passed to search in tvdb.py #8758
Conversation
Currently, if a `list` is passed instead of a single `str`, it will fail the regex match
|
What was SC doing at the time the |
| names.append(re.sub(r"[. -_]", " ", test.group(1)).strip()) | ||
|
|
||
| for attempt in set(n for n in names if n.strip()): | ||
| if isinstance(name, str): |
There was a problem hiding this comment.
Shouldn't this join the list like the other guessit fixes instead of just checking if it's a string?
On line 1 add:
try:
from collections import Iterable
except (ModuleNotFoundError, ImportError):
# Iterable moved in python3.10
from collections.abc import IterableAfter line 106 add:
if isinstance(name, Iterable) and not isinstance(name, (str, bytes);
name = " ".join(name)Then remove the rest of the changes here (we know it's a string then)
WalkthroughThe update involves enhancing the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (1)
sickchill/show/indexers/tvdb.py (1)
Line range hint
194-194: Avoid using a bareexceptclause.- except: + except (requests.exceptions.RequestException, requests.exceptions.HTTPError, Exception) as e: + logger.error(f"Error during search: {str(e)}")Refine the exception handling to catch specific exceptions. This change will help in identifying the exact nature of errors and improve the maintainability of the code.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sickchill/show/indexers/tvdb.py (1 hunks)
Additional Context Used
Ruff (1)
sickchill/show/indexers/tvdb.py (1)
194-194: Do not use bare
except
| for n in name: | ||
| result.extend(self.search(n, language, exact, indexer_id)) |
There was a problem hiding this comment.
Recursive call for list handling might lead to performance issues.
Consider using a loop to handle each item in the list within the same method instance to avoid the overhead of recursive calls. This approach can also make the method easier to debug and maintain.
| break | ||
| except (requests.exceptions.RequestException, requests.exceptions.HTTPError, Exception): | ||
| logger.debug(traceback.format_exc()) | ||
| else: |
There was a problem hiding this comment.
Also this else needs removed. We don't want to search for ["fast", "&", "furious"] each individually, we want to join that list and search for the full string.
|
What about instead enabling mypy type checking and avoid a series of checks used probably by a single caller?
Nicola Canepa
Il giorno 12 mag 2024, 14:41, alle ore 14:41, miigotu ***@***.***> ha scritto:
…
@miigotu commented on this pull request.
> + names.append(test.group(1).strip())
+ # Name with spaces
+ if re.match(r"[. -_]", name):
+ names.append(re.sub(r"[. -_]", " ",
name).strip())
+ if test:
+ # Name with spaces and without year
+ names.append(re.sub(r"[. -_]", " ",
test.group(1)).strip())
+
+ for attempt in set(n for n in names if n.strip()):
+ try:
+ result = self._search(attempt,
language=language)
+ if result:
+ break
+ except (requests.exceptions.RequestException,
requests.exceptions.HTTPError, Exception):
+ logger.debug(traceback.format_exc())
+ else:
Also this else needs removed. We don't want to search for ["fast", "&",
"furious"] each individually, we want to join that list and search for
the full string.
--
Reply to this email directly or view it on GitHub:
#8758 (review)
You are receiving this because you authored the thread.
Message ID:
***@***.***>
|
|
This has been fixed in #8759 |
Currently, if a
listis passed instead of a singlestr, it will fail the regex matchFixes #
Proposed changes in this pull request:
process each item if a list is passed
PR is based on the DEVELOP branch
Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review
Read contribution guide