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

MAINT: align daal4py, onedal and sklearnex modules with isort and black #1375

Merged

Conversation

samir-nasibli
Copy link
Contributor

@samir-nasibli samir-nasibli commented Jul 24, 2023

Description

Advantage:

  • Aligned with isort and black whole proj.
  • This helps avoid messing up new PRs with refactoring and makes review easier.

Commit message will be "apply isort and black fixes only for daal4py, onedal and sklearnex modules"

Disadvantage:

  • this messing up files blame

@ahuber21
Copy link
Contributor

Actually normalizing the entire project is something that I wanted to avoid. I think the commit history in rarely changed files (e.g. the code generators) is more important than code formatting.
Also, why --skip-string-normalization ?

@samir-nasibli
Copy link
Contributor Author

Actually normalizing the entire project is something that I wanted to avoid. I think the commit history in rarely changed files (e.g. the code generators) is more important than code formatting. Also, why --skip-string-normalization ?

As I mentioned on the description, it seems to me that we can do it once. And we will get the commit message for refactoring in all files.
In this case I am just trying to avoid the mess up like on #1374 for sklearnex/_device_offload.py and tests/run_examples.py

@samir-nasibli samir-nasibli changed the title MAINT: align whole project with isort and black MAINT: align daal4py, onedal and sklearnex modules with isort and black Jul 25, 2023
@samir-nasibli
Copy link
Contributor Author

/intelci: run

@samir-nasibli
Copy link
Contributor Author

I am going to do changes in each of sklearnex and onedal module py files: refactoring testing, dpnp integration and I will touch each py file for the algo and utils, so that's why I need codestyle it before. I think further PRs will be mess up with code formatting in other case.

Copy link
Contributor

@Vika-F Vika-F left a comment

Choose a reason for hiding this comment

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

I have not reviewed all 200+ files.
But I think the changes are good as soon as they made by automated code formatting tool and passed the testing.
Let those go to master.

@samir-nasibli samir-nasibli merged commit 14d3132 into intel:master Jul 26, 2023
20 checks passed
@samir-nasibli samir-nasibli deleted the maint/aling_with_black_and_isort branch July 26, 2023 11:35
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