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

Use a max line length of 80 universally #1552

Merged
merged 25 commits into from
Apr 7, 2023

Conversation

ID6109
Copy link
Contributor

@ID6109 ID6109 commented Mar 20, 2023

What does this PR do?

Fixes #1532 + a LOT of typos (including replacing double spaces after every sentence with a single space).

Do let me know if any changes are necessary. Thanks.

PS: I'll go ahead with #1500 after this gets merged. Might be too big of a PR or else.

Who can review?

@LukeWood @ianstenbit

…MaxLineLength

� Conflicts:
�	keras_cv/layers/feature_pyramid.py
�	keras_cv/layers/object_detection/roi_align.py
�	keras_cv/layers/object_detection/rpn_label_encoder.py
�	keras_cv/layers/object_detection_3d/center_net_label_encoder.py
�	keras_cv/layers/object_detection_3d/heatmap_decoder.py
�	keras_cv/layers/preprocessing/base_image_augmentation_layer.py
�	keras_cv/layers/preprocessing/random_channel_shift.py
�	keras_cv/layers/preprocessing/random_translation.py
�	keras_cv/layers/preprocessing_3d/base_augmentation_layer_3d.py
�	keras_cv/layers/regularization/squeeze_excite.py
�	keras_cv/layers/spatial_pyramid.py
�	keras_cv/losses/penalty_reduced_focal_loss.py
�	keras_cv/models/convmixer.py
�	keras_cv/models/csp_darknet.py
�	keras_cv/models/efficientnet_lite.py
�	keras_cv/models/mobilenet_v3.py
�	keras_cv/models/object_detection/faster_rcnn.py
�	keras_cv/models/vit.py
…MaxLineLength

� Conflicts:
�	keras_cv/utils/target_gather.py
…MaxLineLength

� Conflicts:
�	keras_cv/models/backbones/resnet_v2/resnet_v2_backbone.py
…MaxLineLength

� Conflicts:
�	keras_cv/layers/preprocessing/mix_up.py
…MaxLineLength

� Conflicts:
�	keras_cv/callbacks/waymo_evaluation_callback.py
�	keras_cv/layers/preprocessing/base_image_augmentation_layer.py
�	keras_cv/layers/preprocessing/cut_mix.py
�	keras_cv/layers/preprocessing/mosaic.py
�	keras_cv/models/object_detection/retina_net/retina_net_coco_test.py
…MaxLineLength

� Conflicts:
�	keras_cv/metrics/coco/mean_average_precision.py
�	keras_cv/metrics/coco/numerical_tests/mean_average_precision_test.py
�	keras_cv/metrics/coco/numerical_tests/recall_correctness_test.py
�	keras_cv/metrics/coco/recall.py
�	keras_cv/metrics/coco/utils.py
�	keras_cv/utils/fill_utils.py
@ID6109
Copy link
Contributor Author

ID6109 commented Mar 28, 2023

Ping @ianstenbit @LukeWood

@haifeng-jin
Copy link
Contributor

Hi @ID6109,
So sorry for the late reply!

I have resolved the conflicts with master.
However, there are still some new created files have long lines.
Would you help us format them?

Whenever, you have done it, please ping me.
I will help you to merge this PR ASAP to avoid more conflicts and changes.

Copy link
Contributor

@haifeng-jin haifeng-jin left a comment

Choose a reason for hiding this comment

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

This PR is amazing! Thanks for the hard work!

I reviewed part of the PR, more reviews on the way.

benchmarks/vectorized_mosaic.py Outdated Show resolved Hide resolved
keras_cv/layers/fusedmbconv.py Outdated Show resolved Hide resolved
keras_cv/layers/preprocessing/repeated_augmentation.py Outdated Show resolved Hide resolved
keras_cv/layers/regularization/drop_path.py Outdated Show resolved Hide resolved
keras_cv/layers/regularization/dropblock_2d.py Outdated Show resolved Hide resolved
keras_cv/layers/regularization/stochastic_depth.py Outdated Show resolved Hide resolved
keras_cv/losses/giou_loss.py Outdated Show resolved Hide resolved
Copy link
Contributor

@haifeng-jin haifeng-jin left a comment

Choose a reason for hiding this comment

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

More reviews on the way.

keras_cv/losses/iou_loss.py Outdated Show resolved Hide resolved
keras_cv/losses/penalty_reduced_focal_loss.py Outdated Show resolved Hide resolved
keras_cv/models/__internal__/darknet_utils.py Outdated Show resolved Hide resolved
keras_cv/models/backbones/resnet_v2/resnet_v2_backbone.py Outdated Show resolved Hide resolved
keras_cv/models/backbones/resnet_v2/resnet_v2_backbone.py Outdated Show resolved Hide resolved
keras_cv/models/efficientnet_v1.py Outdated Show resolved Hide resolved
keras_cv/models/efficientnet_v1.py Outdated Show resolved Hide resolved
keras_cv/models/efficientnet_v1.py Outdated Show resolved Hide resolved
keras_cv/models/efficientnet_v2.py Outdated Show resolved Hide resolved
keras_cv/models/efficientnet_v2.py Outdated Show resolved Hide resolved
Copy link
Contributor

@haifeng-jin haifeng-jin left a comment

Choose a reason for hiding this comment

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

Review finished.
Thanks again for the PR!

keras_cv/models/mlp_mixer.py Outdated Show resolved Hide resolved
keras_cv/models/mobilenet_v3.py Outdated Show resolved Hide resolved
keras_cv/models/mobilenet_v3.py Outdated Show resolved Hide resolved
keras_cv/models/mobilenet_v3.py Outdated Show resolved Hide resolved
keras_cv/models/vgg16.py Outdated Show resolved Hide resolved
keras_cv/models/vgg19.py Outdated Show resolved Hide resolved
keras_cv/models/vgg19.py Outdated Show resolved Hide resolved
keras_cv/models/vit.py Outdated Show resolved Hide resolved
keras_cv/models/vit.py Outdated Show resolved Hide resolved
@ID6109
Copy link
Contributor Author

ID6109 commented Apr 5, 2023

Hey @haifeng-jin , thanks for the review! It seems there are loads of corrections required in the PR. I'll get back to you once I mitigate them. I'll also go through the codebase once again to see if any similar changes are necessary.

@ID6109 ID6109 requested a review from haifeng-jin April 5, 2023 20:56
Copy link
Contributor

@haifeng-jin haifeng-jin left a comment

Choose a reason for hiding this comment

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

Thanks for the update! a few more comments and we are ready to go.

benchmarks/vectorized_random_sharpness.py Outdated Show resolved Hide resolved
keras_cv/bounding_box/iou.py Outdated Show resolved Hide resolved
keras_cv/datasets/pascal_voc/load.py Outdated Show resolved Hide resolved
keras_cv/layers/fusedmbconv.py Outdated Show resolved Hide resolved
keras_cv/layers/mbconv.py Outdated Show resolved Hide resolved
keras_cv/visualization/draw_bounding_boxes.py Outdated Show resolved Hide resolved
keras_cv/models/vit.py Outdated Show resolved Hide resolved
keras_cv/models/regnet.py Outdated Show resolved Hide resolved
keras_cv/models/object_detection/retina_net/retina_net.py Outdated Show resolved Hide resolved
keras_cv/layers/fusedmbconv.py Outdated Show resolved Hide resolved
@ID6109
Copy link
Contributor Author

ID6109 commented Apr 7, 2023

Thanks, @haifeng-jin . I really appreciate you taking the time to review my code. Seems my understanding of how docstrings are interpreted required some much-needed improvement.

@ID6109 ID6109 requested a review from haifeng-jin April 7, 2023 14:05
Copy link
Contributor

@haifeng-jin haifeng-jin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! ready to be merged

@haifeng-jin
Copy link
Contributor

/gcbrun

@jbischof
Copy link
Contributor

jbischof commented Apr 7, 2023

Thanks for the amazing effort @ID6109!

@haifeng-jin haifeng-jin merged commit 3aaeefc into keras-team:master Apr 7, 2023
@ID6109 ID6109 deleted the MaxLineLength branch April 7, 2023 18:41
@ID6109 ID6109 mentioned this pull request Apr 18, 2023
ghost pushed a commit to y-vectorfield/keras-cv that referenced this pull request Nov 16, 2023
* Enforces line length 80 Part 1

* Enforces line length 80 Part 2 and fixes typos

* Enforces line length 80 Part 3 and fixes typos

* Enforces line length 80 Part 4 and fixes typos

* Enforces line length 80 Part 5 and fixes typos

* Minor change

* Replaced double spaces with a single space

* Merge Conflicts

* Minor Changes

* Resolves Merge Conflicts

* Additional improvements + Changes requested

* Merging

* Minor improvements

---------

Co-authored-by: Your Name <[email protected]>
Co-authored-by: Haifeng Jin <[email protected]>
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.

Use a max line length of 80 universally, and allowlist docstrings with # noqa: E501
3 participants