Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scribblehub: fix author, add tags and synopsis #2374

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Mause
Copy link

@Mause Mause commented May 12, 2024

No description provided.

@Mause Mause closed this May 13, 2024
@Mause Mause reopened this May 13, 2024
@needKVAS
Copy link
Contributor

needKVAS commented May 20, 2024

  1. Now you have def read_novel_info(self) -> BeautifulSoup: and def read_novel_info_in_scraper(self) -> None: which actually returns BeautifulSoup. I think you should revert the first one and fix the second one.

  2. I think you should do something like:

class GeneralBrowserTemplate(BasicBrowserTemplate, GeneralSoupTemplate):
    # ...

    def read_novel_info_in_scraper(self) -> None:
        # ...
        self.parse_additional_info(soup)
        # ...

    def read_novel_info_in_browser(self) -> None:
        # ...
        self.parse_additional_info_in_browser()
        # ...
    
    def parse_additional_info_in_browser() -> None:
        self.parse_additional_info(self.browser.soup)
        
    def parse_additional_info(self, soup: BeautifulSoup) -> None:
        pass
        
    # ...

to support browser. This option can also be used in other classes that inherit GeneralBrowserTemplate.
I find it counterintuitive that
In BasicBrowserTemplate: def read_novel_info_in_scraper(self) -> None:
In GeneralBrowserTemplate: def read_novel_info_in_scraper(self) -> BeautifulSoup:
In ScribbleHubCrawler: def read_novel_info_in_scraper(self) -> None:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants