-
Notifications
You must be signed in to change notification settings - Fork 0
Sourcery refactored master branch #1
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to GitHub API limits, only the first 60 comments can be shown.
| VALID_MANUFACTURERS = set([car["manufacturer"] | ||
| for car in cars.values()]) | ||
| VALID_MANUFACTURERS = {car["manufacturer"] for car in cars.values()} | ||
| CAR_NOT_FOUND = 'Car not found' | ||
|
|
||
| # definition | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 17-22 refactored with the following changes:
- Replace list(), dict() or set() with comprehension (
collection-builtin-to-comprehension) - Replace unneeded comprehension with generator (
comprehension-to-generator)
This removes the following comments ( why? ):
# definition
| """Test to fail create_car's len(cars)+1 (fix max(cars.keys())+1)""" | ||
| car_count = len(cars) | ||
| response = client.delete(f'/99/') | ||
| response = client.delete('/99/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function test_create_car_after_delete refactored with the following changes:
- Replace f-string with no interpolated values with string (
remove-redundant-fstring)
| from program import app | ||
|
|
||
| time_now = str(datetime.today()) | ||
| time_now = str(datetime.now()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 8-8 refactored with the following changes:
- Replace datetime.datetime.today() with datetime.datetime.now() (
use-datetime-now-not-today)
| r = requests.get('https://pokeapi.co/api/v2/pokemon-color/' + colour.lower()) | ||
| r = requests.get(f'https://pokeapi.co/api/v2/pokemon-color/{colour.lower()}') | ||
| pokedata = r.json() | ||
| pokemon = [] | ||
|
|
||
| for i in pokedata['pokemon_species']: | ||
| pokemon.append(i['name']) | ||
|
|
||
| return pokemon | ||
| return [i['name'] for i in pokedata['pokemon_species']] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_poke_colours refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension) - Use f-string instead of string concatenation (
use-fstring-for-concatenation) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| services.location_service.use_cached_data = True | ||
|
|
||
| print("Using cached data? {}".format(data.get('use_cached_data', False))) | ||
| print(f"Using cached data? {data.get('use_cached_data', False)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function configure_app refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| services.location_service.use_cached_data = True | ||
|
|
||
| print("Using cached data? {}".format(data.get('use_cached_data', False))) | ||
| print(f"Using cached data? {data.get('use_cached_data', False)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function configure_app refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| data = resp.json() | ||
|
|
||
| city_data = data.get(f'{zip_code}, {country}', dict()) | ||
| city_data = data.get(f'{zip_code}, {country}', {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_lat_long refactored with the following changes:
- Replace
dict()with{}(dict-literal)
| chose_it = try_int(input('Which one do you want? ')) - 1 | ||
|
|
||
| if not (0 <= chose_it or chose_it < len(scooters)): | ||
| if chose_it < 0 and chose_it >= len(scooters): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function rent_a_scooter refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan) - Ensure constant in comparison is on the right (
flip-comparison)
| return | ||
|
|
||
| conn_str = 'sqlite:///' + db_folder.get_full_path(db_name) | ||
| conn_str = f'sqlite:///{db_folder.get_full_path(db_name)}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function global_init refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation)
| self._falling_through = False | ||
|
|
||
| if isinstance(key, list) or isinstance(key, range): | ||
| if isinstance(key, (list, range)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function switch.case refactored with the following changes:
- Merge isinstance calls (
merge-isinstance) - Replace call to format with f-string (
use-fstring-for-formatting)
| raise Exception("Value does not match any case and there " | ||
| "is no default case: value {}".format(self.value)) | ||
| raise Exception( | ||
| f"Value does not match any case and there is no default case: value {self.value}" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function switch.__exit__ refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
|
|
||
| # noinspection PyComparisonWithNone | ||
| scooters = session.query(Scooter).filter(Scooter.location_id == None).all() | ||
| scooters = session.query(Scooter).filter(Scooter.location_id is None).all() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function rented_scooters refactored with the following changes:
- Use x is None rather than x == None (
none-compare)
| chose_it = try_int(input('Which one do you want? ')) - 1 | ||
|
|
||
| if not (0 <= chose_it or chose_it < len(scooters)): | ||
| if chose_it < 0 and chose_it >= len(scooters): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function rent_a_scooter refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan) - Ensure constant in comparison is on the right (
flip-comparison)
| parked_scooters = [] | ||
| # todo show parked scooters | ||
| print() | ||
| return parked_scooters | ||
| return [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function find_available_scooters refactored with the following changes:
- Move assignment closer to its usage within a block (
move-assign-in-block) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| self._falling_through = False | ||
|
|
||
| if isinstance(key, list) or isinstance(key, range): | ||
| if isinstance(key, (list, range)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function switch.case refactored with the following changes:
- Merge isinstance calls (
merge-isinstance) - Replace call to format with f-string (
use-fstring-for-formatting)
| parts = [ | ||
| p.strip() | ||
| for p in text.split('|') | ||
| if p and p.strip() | ||
| ] | ||
|
|
||
| return parts | ||
| return [p.strip() for p in text.split('|') if p and p.strip()] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function __split_separated_text refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
| def search_by_keyword(_, resp: responder.Response, keyword: str): | ||
| movies = db.search_keyword(keyword) | ||
| print("Searching for movie by keyword: {}".format(keyword)) | ||
| print(f"Searching for movie by keyword: {keyword}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function search_by_keyword refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| def search_by_director(_, resp: responder.Response, director_name: str): | ||
| movies = db.search_director(director_name) | ||
| print("Searching for movie by director: {}".format(director_name)) | ||
| print(f"Searching for movie by director: {director_name}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function search_by_director refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
|
|
||
| def __repr__(self): | ||
| return 'User {}'.format(self.username) | ||
| return f'User {self.username}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function User.__repr__ refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| user = User.query.filter_by(username=request.form.get('username')).first() | ||
| if user: | ||
| if user := User.query.filter_by( | ||
| username=request.form.get('username') | ||
| ).first(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function loginpage refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
| if __name__ == '__main__': | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 57-58 refactored with the following changes:
- Remove redundant conditional (
remove-redundant-if)
| conn_str = 'sqlite:///' + db_file | ||
| print("Connecting to DB at: {}".format(conn_str)) | ||
| conn_str = f'sqlite:///{db_file}' | ||
| print(f"Connecting to DB at: {conn_str}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function DbSession.global_init refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation) - Replace call to format with f-string (
use-fstring-for-formatting)
| repository.add_payment(amount, bill_id) | ||
|
|
||
| return HTTPFound(location='/bill/{}'.format(bill_id)) | ||
| return HTTPFound(location=f'/bill/{bill_id}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function details_post refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| chose_it = try_int(input('Which one do you want? ')) - 1 | ||
|
|
||
| if not (0 <= chose_it or chose_it < len(scooters)): | ||
| if chose_it < 0 and chose_it >= len(scooters): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function rent_a_scooter refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan) - Ensure constant in comparison is on the right (
flip-comparison)
| return | ||
|
|
||
| conn_str = 'sqlite:///' + db_folder.get_full_path(db_name) | ||
| conn_str = f'sqlite:///{db_folder.get_full_path(db_name)}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function global_init refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation)
| return HTTPFound(location='/bill/{}'.format(vm.bill_id)) | ||
| return HTTPFound(location=f'/bill/{vm.bill_id}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function details_post refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| conn_str = 'sqlite:///' + db_file | ||
| print("Connecting to DB at: {}".format(conn_str)) | ||
| conn_str = f'sqlite:///{db_file}' | ||
| print(f"Connecting to DB at: {conn_str}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function DbSession.global_init refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation) - Replace call to format with f-string (
use-fstring-for-formatting)
|
|
||
| if not self.user: | ||
| self.error = "No user with ID {}.".format(user_id) | ||
| self.error = f"No user with ID {user_id}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function IndexViewModel.__init__ refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| return Response(status=404) | ||
|
|
||
| return vm.to_dict() | ||
| return Response(status=404) if not vm.bill else vm.to_dict() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function details_get refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
| return HTTPFound(location='/bill/{}'.format(vm.bill_id)) | ||
| return HTTPFound(location=f'/bill/{vm.bill_id}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function details_post refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
Branch
masterrefactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
masterbranch, then run:Help us improve this pull request!