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

Migrate pylint check to Python 3 #4381

Merged
merged 4 commits into from
Apr 21, 2019
Merged

Migrate pylint check to Python 3 #4381

merged 4 commits into from
Apr 21, 2019

Conversation

hcho3
Copy link
Collaborator

@hcho3 hcho3 commented Apr 17, 2019

As we are about to deprecate Python 2 (#4379), we need to migrate pylint (Python coding style check) to Python 3. There are some minor changes to meet the new requirements.

@@ -1721,7 +1714,7 @@ def get_split_value_histogram(self, feature, fmap='', bins=None, as_pandas=True)
xgdump = self.get_dump(fmap=fmap)
values = []
regexp = re.compile(r"\[{0}<([\d.Ee+-]+)\]".format(feature))
for i in range(len(xgdump)):
for i, _ in enumerate(xgdump):
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, why using enumerate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To comply with pylint style guideline: pylint-dev/pylint#684

@@ -462,7 +462,7 @@ def cv(params, dtrain, num_boost_round=10, nfold=3, stratified=False, folds=None
rank=0,
evaluation_result_list=res))
except EarlyStopException as e:
for k in results.keys():
for k in results:
Copy link
Member

Choose a reason for hiding this comment

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

Does this change anything related to code logic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, the change is to comply with the latest pylint guideline (pylint-dev/pylint#699). Logic-wise nothing changed.

@hcho3
Copy link
Collaborator Author

hcho3 commented Apr 21, 2019

@trivialfis I think this is ready. I'd like to get this merged soon, since #4380 depends on it.

Copy link
Member

@trivialfis trivialfis left a comment

Choose a reason for hiding this comment

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

Go ahead once the tests are fixed.

@hcho3 hcho3 merged commit bbe0dbd into dmlc:master Apr 21, 2019
@hcho3 hcho3 deleted the python3_lint branch April 21, 2019 08:01
CodingCat pushed a commit to CodingCat/xgboost that referenced this pull request Apr 22, 2019
* Migrate lint to Python 3

* Fix lint errors

* Use Miniconda3 to use Python 3.7

* Use latest pylint and astroid
@lock lock bot locked as resolved and limited conversation to collaborators Jul 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants