Fix formats in fastapi/utils #5075
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5075 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 532 532
Lines 13672 13670 -2
=========================================
- Hits 13672 13670 -2
Continue to review full report at Codecov.
|
|
📝 Docs preview for commit 0295250 at: https://62b5c5ec9920ad03052f547d--fastapi.netlify.app |
|
📝 Docs preview for commit 66a7b7b at: https://62b5cde13f6bab09bf7e0273--fastapi.netlify.app |
There was a problem hiding this comment.
I like the changes to the string formatting (e.g using f"blaat{var}" as it is more widely used in other parts of FastAPI. There is one more instance I found you could add:
fastapi/fastapi/dependencies/utils.py
Line 724 in 1876ebc
However, the changes on line 54-56 are less explicit than what they are replacing. I find it better to set variables explicitly rather than inline when passing them as parameters in another method call (which you are proposing). The same goes for line 172/173; I like it better on two lines than in a single statement in a for-statement.
fastapi/utils.py
Outdated
| return operation_id | ||
|
|
||
|
|
||
| def generate_unique_id(route: "APIRoute") -> str: | ||
| operation_id = route.name + route.path_format | ||
| operation_id = re.sub("[^0-9a-zA-Z_]", "_", operation_id) | ||
| assert route.methods | ||
| operation_id = operation_id + "_" + list(route.methods)[0].lower() | ||
| operation_id = f"{operation_id}_{list(route.methods)[0].lower()}" |
There was a problem hiding this comment.
This is proper, it is in line with other LoC in this package so I would be supporting this.
There was a problem hiding this comment.
re.compile("[^0-9a-zA-Z_]") or re.compile("\W") for speedup
| @@ -134,15 +132,15 @@ def generate_operation_id_for_path( | |||
| ) | |||
| operation_id = name + path | |||
| operation_id = re.sub("[^0-9a-zA-Z_]", "_", operation_id) | |||
| operation_id = operation_id + "_" + method.lower() | |||
| operation_id = f"{operation_id}_{method.lower()}" | |||
There was a problem hiding this comment.
And therefor, I would also be supporting it here.
|
📝 Docs preview for commit 98e137a at: https://62bc72dd7b439700aec3cff2--fastapi.netlify.app |
|
📝 Docs preview for commit d84df5c at: https://62bda3a4f0a8b20da4ff294c--fastapi.netlify.app |
Fix & improve formats in utils