keras icon indicating copy to clipboard operation
keras copied to clipboard

[Refactoring] making the code more Succinct and Pythonic

Open maldil opened this issue 2 years ago • 2 comments

This PR uses the with statement and the .join function to make the code more Pythonic and Succinct.

maldil avatar Aug 06 '22 08:08 maldil

Lint / Check the code format (pull_request) Failing after 2m — Check the code format

Please fix code formatting.

fchollet avatar Aug 08 '22 18:08 fchollet

@fchollet Below is the output of lint check.

Run sh shell/lint.sh
[4](https://github.com/keras-team/keras/runs/7731885042?check_suite_focus=true#step:6:5)
no issues with isort
[5](https://github.com/keras-team/keras/runs/7731885042?check_suite_focus=true#step:6:6)
no issues with flake8
[6](https://github.com/keras-team/keras/runs/7731885042?check_suite_focus=true#step:6:7)
would reformat keras/utils/text_dataset_test.py
[7](https://github.com/keras-team/keras/runs/7731885042?check_suite_focus=true#step:6:8)

[8](https://github.com/keras-team/keras/runs/7731885042?check_suite_focus=true#step:6:9)
Oh no! 💥 💔 💥
[9](https://github.com/keras-team/keras/runs/7731885042?check_suite_focus=true#step:6:10)
1 file would be reformatted, 699 files would be left unchanged.
[10](https://github.com/keras-team/keras/runs/7731885042?check_suite_focus=true#step:6:11)```
Please run "sh shell/format.sh" to format the code.
[11](https://github.com/keras-team/keras/runs/7731885042?check_suite_focus=true#step:6:12)
Error: Process completed with exit code 1.

I ran the sh shell/format.sh as suggested, the formatting issue is keras/utils/text_dataset_test.py:155:13: B007 Loop control variable 'batch' not used within the loop body. If this is intended, start the name with an underscore, which is not introduced by this PR.
As shown below, the variable batch is used below the for loop therefore I feel this is unsafe to fix. https://github.com/keras-team/keras/blob/3a5a21b16916bf4bd0aec4ee92c0fdf8c1556b67/keras/utils/text_dataset_test.py#L155

maldil avatar Aug 08 '22 18:08 maldil

@maldil Can you please resolve conflicts? Thank you!

gbaned avatar Aug 12 '22 16:08 gbaned

@gbaned this is done. please help me to rerun the workflow.

maldil avatar Aug 12 '22 22:08 maldil

This branch cannot be rebased due to conflicts

Please resolve this.

fchollet avatar Aug 13 '22 17:08 fchollet

Lint / Check the code format (pull_request)

Please run black as well.

fchollet avatar Aug 13 '22 17:08 fchollet

@fchollet Running in black introduces the following changes; I'm not sure if these changes should be pushed. I removed the white spaces that were causing the lint failure. Hopefully, it will fix the issue. Screen Shot 2022-08-13 at 7 42 57 PM

maldil avatar Aug 14 '22 02:08 maldil

@gbaned can you please help me to rerun the workflow?

maldil avatar Aug 15 '22 20:08 maldil

You should push the changes made by black. The code in the PR should pass the black lint check.

fchollet avatar Aug 16 '22 17:08 fchollet

This branch cannot be rebased due to conflicts

Please rebase.

fchollet avatar Aug 16 '22 17:08 fchollet

I ran sh shell/format.sh and pushed the changes. Hopefully this will fix the issue.

maldil avatar Aug 16 '22 19:08 maldil

This branch cannot be rebased due to conflicts

The black check passed! But you still need to rebase.

fchollet avatar Aug 16 '22 21:08 fchollet

@fchollet rebased

maldil avatar Aug 17 '22 06:08 maldil