-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
TYP/CLN: cleanup _openpyxl.py
, add type annotation #36021
#36022
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
Conversation
_openpyxl.py
, add type annotation #36021
@@ -515,16 +484,17 @@ def get_sheet_by_index(self, index: int): | |||
|
|||
def _convert_cell(self, cell, convert_float: bool) -> Scalar: | |||
|
|||
# TODO: replace with openpyxl constants | |||
from openpyxl.cell.cell import TYPE_BOOL, TYPE_ERROR, TYPE_NUMERIC | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openpyxl verion < 2.6 would raise import error here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @VirosaLi for the PR.
if updating minimum versions need to update Increased minimum versions for dependencies of doc\source\whatsnew\v1.2.0.rst and pandas\doc\source\getting_started\install.rst
I'm not sure whether we want to update minimum versions just for typing @WillAyd
@VirosaLi to fix the import errors, use quotes around the type (or can use |
return self.book.save(self.path) | ||
|
||
@classmethod | ||
def _convert_to_style(cls, style_dict): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see this method is used anywhere. And it doesn't affect any test. Also, it seems like openpyxl doesn't have a style
module anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments. I think the bump is OK given it's been out for a year and a half at this point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @VirosaLi
…ev#36021 (pandas-dev#36022)" This reverts commit ecc5015.
_openpyxl.py
, add type annotation #36021black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff