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

Improve Python Code Formatting and Quality #94

Merged
merged 4 commits into from
May 2, 2024

Conversation

azriel1rf
Copy link
Collaborator

@azriel1rf azriel1rf commented May 2, 2024

This pull request focuses on enhancing the formatting and quality of the Python code in this project.

The key changes include:

  • Formatted all Python files using the Ruff formatter to ensure consistent and clean code style
    • Replaced typing.List with list and added from __future__ import annotations for better type annotations
  • Manually fixed any remaining linter errors flagged by Ruff
  • Formatted the setup.cfg file using asottile/setup-cfg-fmt for better readability and structure
  • Removed the unnecessary numpy dependency from setup.cfg
  • Updated the license file path in setup.cfg to correctly reference the LICENSE file

For Python Codes

Formatting Python Files

The Ruff formatter was applied to all Python files in the project. Ruff automatically formats the code to adhere to best practices and style guidelines.

Type Annotation Improvements

Previously, we had replaced list with typing.List for type annotations (#90). However, it was discovered that using list with from __future__ import annotations provides a more concise and compatible approach. Therefore, in this pull request, we have reverted back to using list for type annotations. This change resolves the 'type' object is not subscriptable error that was previously encountered (#75).

Manual Linter Error Fixes

After applying the Ruff formatter, any remaining linter errors were manually addressed. This involved making necessary code changes to resolve issues related to naming conventions, eliminating magic numbers by defining them as constants, creating GitHub issues for TODO comments and linking them in the code, rewording comments and other quality aspects.

For setup.cfg

setup.cfg Formatting

The setup.cfg file was formatted using the setup-cfg-fmt tool.

Dependency Removal

The numpy dependency was removed from the setup.cfg file. It was determined that this dependency was not actively used in the project, and removing it helps streamline the project's dependencies, reducing unnecessary overhead.

License File Path Update

The license_files configuration in setup.cfg was updated to accurately point to the LICENSE file.


These changes aim to improve the overall code quality, readability, and maintainability of the Python codebase. By adhering to consistent formatting, resolving linter errors, optimizing the project's configuration, and enhancing type annotations, we can ensure a cleaner and more robust project structure. These changes are compatible with future linter and formatter standards in this repo. (#95)

Please review the changes and provide any feedback or suggestions.
Thank you for your consideration!

)

* Refactor card_sampler include paths in benchmark files

* Refactor to use `card_sampler::CardSampler` in tests

* Refactor card_sampler implementation to separate header and source files
Copy link
Owner

@HenryRLee HenryRLee left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for cleaning the format!

@HenryRLee HenryRLee merged commit 8de95ab into develop May 2, 2024
6 checks passed
azriel1rf added a commit that referenced this pull request May 4, 2024
* Fix setup.cfg

* Refactor Python codes with Ruff and manual fixing.
azriel1rf added a commit that referenced this pull request May 4, 2024
* Fix setup.cfg

* Refactor Python codes with Ruff and manual fixing.
@azriel1rf azriel1rf deleted the refactor_python_files branch May 5, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants