Skip to content

Simplified and faster encoding.py & doc building - #138

Merged
buriy merged 6 commits into
buriy:masterfrom
adbar:master
Sep 14, 2021
Merged

buriy merged 6 commits into
buriy:masterfrom
adbar:master

Conversation

@adbar

@adbar adbar commented Feb 7, 2020

Copy link
Copy Markdown
  • code cleaning: regexes + LookupError unnecessary as fix_charset() returns a default value
  • faster encoding detection with cchardet, made optional in encoding.py and setup.py

@adbar

adbar commented Feb 7, 2020

Copy link
Copy Markdown
Author

The conflict comes from setup.py, please review it

Comment thread readability/encoding.py
@adbar

adbar commented Feb 19, 2020

Copy link
Copy Markdown
Author
  • added bypass of doc_build

@adbar adbar changed the title Simplified and faster encoding.py Simplified and faster encoding.py & doc building Feb 19, 2020
def _parse(self, input):
doc, self.encoding = build_doc(input)
if isinstance(input, (_ElementTree, HtmlElement)):
doc = input

@buriy buriy Feb 19, 2020 •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option doesn't set self.encoding which was set in another branch.
Users might be relying on it.
So please add
self.endoding = 'utf-8'
to match the expectations in the best way and we'll publish it then.

@buriy buriy Feb 19, 2020 •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, you have no deepcopy as we have discussed.
Any thoughts on this?
._parse method could be run several times, devastating the document after the first iteration and thus reducing the found subtree quality.
See

and
return shorten_title(self._html(True))

and etc.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the encoding attribute, for the deepcopy issue I'm not sure what to do, passing already existing trees is experimental anyway

@adbar

adbar commented Sep 14, 2021

Copy link
Copy Markdown
Author

@buriy Could you please accept or amend the PR? All tests pass and we could keep the deepcopy improvement for later.

@buriy
buriy merged commit 1415c30 into buriy:master Sep 14, 2021
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