Skip to content

Replace flynt checker with pylint consider-using-f-string #6171

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.

Sign up for GitHub

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

Jump to bottom
Closed
pavoljuhas opened this issue Jun 27, 2023 · 3 comments · Fixed by #6164 or #6198
Closed

Replace flynt checker with pylint consider-using-f-string #6171

pavoljuhas opened this issue Jun 27, 2023 · 3 comments · Fixed by #6164 or #6198
Assignees
@pavoljuhas
Labels
kind/health For CI/testing/release process/refactoring/technical debt items triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@pavoljuhas
Copy link
Collaborator

Description of the issue

flynt is a tool for converting %-format and "{}".format() strings to Python f-strings.
It is used in the check/format-incremental script and also as an example for package metadata reading in a unit tests here.

The same functionality can be provided by the consider-using-f-string pylint rule.

I propose to use pylint rule instead of pylint and also replace flynt usage in the unit test with a more common package.

Cirq version

1.2.0.dev at 3f8d83a
@pavoljuhas pavoljuhas added kind/health For CI/testing/release process/refactoring/technical debt items good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. triage/discuss Needs decision / discussion, bring these up during Cirq Cynque and removed good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. no QC knowledge needed Want to contribute to Cirq, but don't know quantum computing? This issue is for you. labels Jun 27, 2023
@pavoljuhas
Copy link
Collaborator Author

pavoljuhas commented Jun 27, 2023 •
edited by tanujkhattar
Loading

@NoureldinYosri NoureldinYosri added the triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on label Jul 5, 2023
@tanujkhattar tanujkhattar removed the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Jul 5, 2023
@pavoljuhas pavoljuhas self-assigned this Jul 5, 2023
@NoureldinYosri
Copy link
Collaborator

cirq-sync: accepted lets remove flynt and use the new dependency instead

@pavoljuhas pavoljuhas mentioned this issue Jul 6, 2023
Merged
pavoljuhas added a commit that referenced this issue Jul 12, 2023
@pavoljuhas @tanujkhattar
Remove dependency on the flynt tool (#6164)
8e70f77 
- pylint - enable `consider-using-f-string` check
- Test package metadata with numpy rather than flynt
- Fix CI complains on test coverage - fix markup for ignoring coverage
- Temporarily disable consider-using-f-string pylint and add TODO-s to fix later
- Format touched files with black-23.3.0

Partially fixes #6171

---------

Co-authored-by: Tanuj Khattar <[email protected]>
@pavoljuhas pavoljuhas reopened this Jul 14, 2023
@pavoljuhas pavoljuhas mentioned this issue Jul 14, 2023
Merged
pavoljuhas added a commit that referenced this issue Jul 17, 2023
@pavoljuhas
Use f-strings instead of percent operator or str.format() (#6198)
084e273 
Convert all `%` and `str.format()` expressions or disable pylint
check where `str.format()` is more readable.

Partially fixes #6171
@pavoljuhas pavoljuhas mentioned this issue Jul 17, 2023
Merged
pavoljuhas added a commit that referenced this issue Jul 17, 2023
@pavoljuhas
Ignore string updates for consider-using-f-string (#6203)
45ed228 
Part of #6171
@tanujkhattar tanujkhattar mentioned this issue Jul 17, 2023
Merged
@viathor
Copy link
Collaborator

viathor commented Jul 17, 2023

dev_tools/pr_monitor.py disables consider-using-f-string at file level. We probably want to re-enable.

@pavoljuhas pavoljuhas mentioned this issue Jul 18, 2023
Merged
pavoljuhas added a commit that referenced this issue Jul 22, 2023
@pavoljuhas
Use f-strings in the pr_monitor tool (#6208)
43d0372 
* pr_monitor - convert to f-strings to satisfy pylint consider-using-f-string

* clean up some redundant fixed strings concatenations

* also pin pytest-cov==3 to unblock CI coverage check which failed
  because pytest-cov==4.1.0 does not see line hits in a subprocess.

Finalizes #6171
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@pavoljuhas @tanujkhattar
Remove dependency on the flynt tool (quantumlib#6164)
b1bfb1c 
- pylint - enable `consider-using-f-string` check
- Test package metadata with numpy rather than flynt
- Fix CI complains on test coverage - fix markup for ignoring coverage
- Temporarily disable consider-using-f-string pylint and add TODO-s to fix later
- Format touched files with black-23.3.0

Partially fixes quantumlib#6171

---------

Co-authored-by: Tanuj Khattar <[email protected]>
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@pavoljuhas
Use f-strings instead of percent operator or str.format() (quantumlib…
db32224 
…#6198)

Convert all `%` and `str.format()` expressions or disable pylint
check where `str.format()` is more readable.

Partially fixes quantumlib#6171
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@pavoljuhas
Use f-strings in the pr_monitor tool (quantumlib#6208)
dec6d9b 
* pr_monitor - convert to f-strings to satisfy pylint consider-using-f-string

* clean up some redundant fixed strings concatenations

* also pin pytest-cov==3 to unblock CI coverage check which failed
  because pytest-cov==4.1.0 does not see line hits in a subprocess.

Finalizes quantumlib#6171
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pavoljuhas pavoljuhas

Labels
kind/health For CI/testing/release process/refactoring/technical debt items triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Milestone
No milestone
4 participants
@pavoljuhas @NoureldinYosri @tanujkhattar @viathor