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

feature/add-optional-kwargs: Added optional kwargs #309

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

098799
Copy link

@098799 098799 commented Dec 11, 2019

Most of the init arguments in the HTML2Text class are hardcoded in
constants and modifiable only by the cli, not through the library
usage. This adds the possibility to pass kwargs through the function
call html2text or class init.

Please note that the commit contains syntax that is not recognizable
by mypy, but is correct. Note: python/mypy#5719

@coveralls
Copy link

coveralls commented Dec 11, 2019

Coverage Status

Coverage decreased (-0.05%) to 97.824% when pulling a6d9f9a on RedPointsSolutions:feature/add-optional-kwargs into 2d2c702 on Alir3z4:master.

@098799 098799 force-pushed the feature/add-optional-kwargs branch 2 times, most recently from 349bc17 to 37ec4fc Compare December 11, 2019 16:15
Most of the init arguments in the ``HTML2Text`` class are hardcoded in
constants and modifiable only by the cli, not through the library
usage. This adds the possibility to pass kwargs through the function
call ``html2text`` or class init.

Please note that the commit contains syntax that is not recognizable
by ``mypy``, but is correct. Note: python/mypy#5719
@Alir3z4
Copy link
Owner

Alir3z4 commented Sep 13, 2020

Most of the init arguments in the HTML2Text class are hardcoded in
constants and modifiable only by the cli, not through the library
usage.

I consider that as a problem, but the approach doesn't look clean to me, regardless of of how mypy works with it, the code has become harder to read, especially when setattr is being used here.

The code would be much easier to read when it's explicit and easily being able to follow.

@098799
Copy link
Author

098799 commented Sep 14, 2020

Hey @Alir3z4, thanks for the reply. If you prefer, I can keep the current structure and replicate the big list once again in the kwargs of __init__. Would you prefer that solution?

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.

3 participants