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

Rework attributes #155

Merged
merged 24 commits into from
Apr 12, 2023
Merged

Rework attributes #155

merged 24 commits into from
Apr 12, 2023

Conversation

MaxDall
Copy link
Collaborator

@MaxDall MaxDall commented Apr 5, 2023

This should only be merged after #148 and with the proper settings. Currently this commits into #148

This PR addresses issues raised in #138.

It does so by adding a parameter supported to the attribute decorator.

  • If set to True (default), the decorated attribute is validated against the attribute_guidelines (name, annotation),
  • If set to False the attribute is excluded from those tests.

This also does a major refactor on the article class. We have to so article supports unsupported attributes.

  • Every parsed attribute is now accessible through Article, but only those supported will be actual fields of Article. See the following example
class TestParser(BaseParser):
    @attribute
    def body(self) -> ArticleBody:
        ...

    @attribute
    def authors(self) -> List[str]:
        ...

    @attribute
    def publishing_date(self) -> Optional[datetime.datetime]:
        ...

    ...

    @attribute(supported=False)
    def key_notes(self):
        ...

You can now access all attributes with dot notation (article.body, article.key_notes) but only those in the guidelines will have auto-completion, and providing no annotation for key_notes here won't raise any error duuring testing.

  • some rework in the Article structure and inheritance.

@MaxDall MaxDall added the DON'T MERGE Current PR is based on another PR. When used, indicate in title with [Based on #...] label Apr 5, 2023
src/parser/html_parser/base_parser.py Outdated Show resolved Hide resolved
src/scraping/article.py Outdated Show resolved Hide resolved
src/scraping/article.py Outdated Show resolved Hide resolved
src/scraping/article.py Outdated Show resolved Hide resolved
src/scraping/article.py Outdated Show resolved Hide resolved
src/scraping/source.py Outdated Show resolved Hide resolved
tests/fixtures/fixture_parser.py Outdated Show resolved Hide resolved
src/scraping/article.py Show resolved Hide resolved
src/parser/html_parser/base_parser.py Outdated Show resolved Hide resolved
@dobbersc
Copy link
Collaborator

I tried my best to review this PR. But be aware that I do not totally understand everything that has been changed.

src/scraping/article.py Outdated Show resolved Hide resolved
src/scraping/article.py Outdated Show resolved Hide resolved
MaxDall and others added 2 commits April 11, 2023 23:39
Co-authored-by: Conrad Dobberstein <29147025+dobbersc@users.noreply.github.com>
Base automatically changed from parse_attribute_annotations to master April 12, 2023 12:08
@dobbersc dobbersc removed the DON'T MERGE Current PR is based on another PR. When used, indicate in title with [Based on #...] label Apr 12, 2023
src/parser/html_parser/base_parser.py Outdated Show resolved Hide resolved
src/scraping/article.py Outdated Show resolved Hide resolved
src/scraping/article.py Outdated Show resolved Hide resolved
Co-authored-by: Conrad Dobberstein <29147025+dobbersc@users.noreply.github.com>
MaxDall and others added 2 commits April 12, 2023 15:03
Co-authored-by: Conrad Dobberstein <29147025+dobbersc@users.noreply.github.com>
Co-authored-by: Conrad Dobberstein <29147025+dobbersc@users.noreply.github.com>
@dobbersc
Copy link
Collaborator

Ah there is one parameter left to rename

@dobbersc dobbersc merged commit 8a69b02 into master Apr 12, 2023
@dobbersc dobbersc deleted the rework_attributes branch April 12, 2023 13:06
@MaxDall MaxDall mentioned this pull request Apr 26, 2023
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.

None yet

2 participants