Sourcery refactored develop branch #1
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
| data = { | ||
| # The x series is made of random numbers between 1 and 10 | ||
| "x": [random.uniform(1, 10) for i in y], | ||
| "y": y, | ||
| } | ||
| data = {"x": [random.uniform(1, 10) for _ in y], "y": y} | ||
|
|
||
| options = { | ||
| "error_x": { | ||
| "type": "data", | ||
| # Allows for a 'plus' and a 'minus' error data | ||
| "symmetric": False, | ||
| # The 'plus' error data is a series of random numbers | ||
| "array": [random.uniform(0, 5) for i in y], | ||
| # The 'minus' error data is a series of random numbers | ||
| "arrayminus": [random.uniform(0, 2) for i in y], | ||
| # Color of the error bar | ||
| "array": [random.uniform(0, 5) for _ in y], | ||
| "arrayminus": [random.uniform(0, 2) for _ in y], |
There was a problem hiding this comment.
Lines 25-40 refactored with the following changes:
- Replace unused for index with underscore [×3] (
for-index-underscore)
This removes the following comments ( why? ):
# The 'minus' error data is a series of random numbers
# Color of the error bar
# The 'plus' error data is a series of random numbers
# Allows for a 'plus' and a 'minus' error data
# The x series is made of random numbers between 1 and 10
| data = [random.random() for i in range(500)] | ||
| data = [random.random() for _ in range(500)] |
There was a problem hiding this comment.
Lines 21-21 refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore)
| data = {"Count": [random.random() for i in range(100)]} | ||
| data = {"Count": [random.random() for _ in range(100)]} |
There was a problem hiding this comment.
Lines 21-21 refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore)
| samples = {"x": [random.gauss() for i in range(100)]} | ||
| samples = {"x": [random.gauss() for _ in range(100)]} |
There was a problem hiding this comment.
Lines 21-21 refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore)
| data = [random.random() for i in range(100)] | ||
| data = [random.random() for _ in range(100)] |
There was a problem hiding this comment.
Lines 21-21 refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore)
| else: | ||
| if not ( | ||
| (isinstance(config_value, List) or isinstance(config_value, Set)) | ||
| and all(map(lambda x: isinstance(x, child_config_class), config_value)) | ||
| ): | ||
| self._error( | ||
| config_key, | ||
| config_value, | ||
| f"{config_key} field of {parent_config_class.__name__} `{config_id}` must be populated with a list " | ||
| f"of {child_config_class.__name__} objects.", | ||
| ) | ||
| elif not ( | ||
| (isinstance(config_value, (List, Set))) | ||
| and all(map(lambda x: isinstance(x, child_config_class), config_value)) | ||
| ): | ||
| self._error( | ||
| config_key, | ||
| config_value, | ||
| f"{config_key} field of {parent_config_class.__name__} `{config_id}` must be populated with a list " | ||
| f"of {child_config_class.__name__} objects.", | ||
| ) |
There was a problem hiding this comment.
Function _ConfigChecker._check_children refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif) - Merge isinstance calls (
merge-isinstance)
| cls.__logger.error("ConfigurationUpdateBlocked: " + error_message) | ||
| cls.__logger.error(f"ConfigurationUpdateBlocked: {error_message}") |
There was a problem hiding this comment.
Function _ConfigBlocker._check refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation)
| match = re.fullmatch(cls._PATTERN, str(template)) | ||
| if match: | ||
| if match := re.fullmatch(cls._PATTERN, str(template)): |
There was a problem hiding this comment.
Function _TemplateHandler._replace_template refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
| return str.lower(val) == "true" or not (str.lower(val) == "false") | ||
| return str.lower(val) == "true" or str.lower(val) != "false" |
There was a problem hiding this comment.
Function _TemplateHandler._to_bool refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan)
| return key[:5] + "taipy-" + key[5:] | ||
| return f"{key[:5]}taipy-{key[5:]}" | ||
|
|
||
| return key[:2] + "taipy-" + key[2:] | ||
| return f"{key[:2]}taipy-{key[2:]}" |
There was a problem hiding this comment.
Function _CoreCLI.__add_taipy_prefix refactored with the following changes:
- Use f-string instead of string concatenation [×4] (
use-fstring-for-concatenation)
| if parent_entity._MANAGER_NAME in current_parent_dict.keys(): | ||
| if parent_entity._MANAGER_NAME in current_parent_dict: |
There was a problem hiding this comment.
Function get_parents refactored with the following changes:
- Remove unnecessary call to keys() (
remove-dict-keys)
| if scenario.cycle in cycles_scenarios.keys(): | ||
| if scenario.cycle in cycles_scenarios: |
There was a problem hiding this comment.
Function get_cycles_scenarios refactored with the following changes:
- Remove unnecessary call to keys() (
remove-dict-keys)
| if preserve_file_path := os.getenv(__BACKUP_FILE_PATH_ENVIRONMENT_VARIABLE_NAME, None): | ||
| storage_folder = os.path.abspath(Config.core.storage_folder) + os.sep | ||
| if not os.path.abspath(to_remove_file_path).startswith(storage_folder): | ||
| try: | ||
| with open(preserve_file_path, "r+") as f: | ||
| old_backup = f.read() | ||
| to_remove_file_path = to_remove_file_path + "\n" | ||
|
|
||
| # To avoid removing the file path of different data nodes that are pointing | ||
| # to the same file. We will only replace the file path only once. | ||
| if old_backup.startswith(to_remove_file_path): | ||
| new_backup = old_backup.replace(to_remove_file_path, "", 1) | ||
| else: | ||
| new_backup = old_backup.replace("\n" + to_remove_file_path, "\n", 1) | ||
|
|
||
| if new_backup is not old_backup: | ||
| f.seek(0) | ||
| f.write(new_backup) | ||
| f.truncate() | ||
| except Exception: | ||
| pass | ||
| if not ( | ||
| preserve_file_path := os.getenv( | ||
| __BACKUP_FILE_PATH_ENVIRONMENT_VARIABLE_NAME, None | ||
| ) | ||
| ): | ||
| return | ||
| storage_folder = os.path.abspath(Config.core.storage_folder) + os.sep | ||
| if not os.path.abspath(to_remove_file_path).startswith(storage_folder): | ||
| try: | ||
| with open(preserve_file_path, "r+") as f: | ||
| old_backup = f.read() | ||
| to_remove_file_path += "\n" | ||
|
|
||
| # To avoid removing the file path of different data nodes that are pointing | ||
| # to the same file. We will only replace the file path only once. | ||
| if old_backup.startswith(to_remove_file_path): | ||
| new_backup = old_backup.replace(to_remove_file_path, "", 1) | ||
| else: | ||
| new_backup = old_backup.replace("\n" + to_remove_file_path, "\n", 1) | ||
|
|
||
| if new_backup is not old_backup: | ||
| f.seek(0) | ||
| f.write(new_backup) | ||
| f.truncate() | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Function _remove_from_backup_file refactored with the following changes:
- Add guard clause (
last-if-guard) - Replace assignment with augmented assignment (
aug-assign)
| self._sorted_nodes = list(nodes for nodes in nx.topological_generations(dag)) | ||
| self._sorted_nodes = list(nx.topological_generations(dag)) |
There was a problem hiding this comment.
Function _DAG.__init__ refactored with the following changes:
- Simplify generator expression (
simplify-generator)
| return len(self._sorted_nodes), max([len(i) for i in self._sorted_nodes]) | ||
| return len(self._sorted_nodes), max(len(i) for i in self._sorted_nodes) |
There was a problem hiding this comment.
Function _DAG.__compute_size refactored with the following changes:
- Replace unneeded comprehension with generator (
comprehension-to-generator)
| return list(nodes for nodes in nx.topological_generations(dag) if (Task in (type(node) for node in nodes))) | ||
| return [ | ||
| nodes | ||
| for nodes in nx.topological_generations(dag) | ||
| if (Task in (type(node) for node in nodes)) | ||
| ] |
There was a problem hiding this comment.
Function Submittable._get_sorted_tasks refactored with the following changes:
- Replace list(), dict() or set() with comprehension (
collection-builtin-to-comprehension)
| elif elem := [x for x in self._subscribers if x.callback == callback]: | ||
| self._subscribers.remove(elem[0]) | ||
| else: | ||
| raise ValueError |
There was a problem hiding this comment.
Function Submittable._remove_subscriber refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif) - Use named expression to simplify assignment and conditional (
use-named-expression) - Lift code into else after jump in control flow (
reintroduce-else) - Swap if/else branches (
swap-if-else-branches)
| if entity_type == "TASK" and "SCENARIO" in _id: | ||
| if entity_id in entity_data["tasks"]: | ||
| if entity_id in entity_data["tasks"]: | ||
| if entity_type == "TASK" and "SCENARIO" in _id: |
There was a problem hiding this comment.
Function __search_parent_ids refactored with the following changes:
- Swap positions of nested conditionals [×3] (
swap-nested-ifs)
| section_id = f"{entity_id}:SECTION" | ||
| for _id, entity_data in data.items(): | ||
| section_id = f"{entity_id}:SECTION" |
There was a problem hiding this comment.
Function __search_parent_config refactored with the following changes:
- Hoist statements out of for/while loops (
hoist-statement-from-loop)
| if entity_type in ["JOB", "VERSION"]: | ||
| if entity_type in {"JOB", "VERSION"}: |
There was a problem hiding this comment.
Function __migrate_entities refactored with the following changes:
- Use set when checking membership of a collection of literals (
collection-into-set)
| for task in ts: | ||
| jobs.append( | ||
| cls._lock_dn_output_and_create_job( | ||
| task, | ||
| submission.id, | ||
| submission.entity_id, | ||
| callbacks=itertools.chain([submission._update_submission_status], callbacks or []), | ||
| force=force, # type: ignore | ||
| ) | ||
| jobs.extend( | ||
| cls._lock_dn_output_and_create_job( | ||
| task, | ||
| submission.id, | ||
| submission.entity_id, | ||
| callbacks=itertools.chain( | ||
| [submission._update_submission_status], callbacks or [] | ||
| ), | ||
| force=force, # type: ignore | ||
| ) | ||
|
|
||
| for task in ts | ||
| ) | ||
| submission.jobs = jobs # type: ignore | ||
|
|
||
| cls._orchestrate_job_to_run_or_block(jobs) | ||
|
|
||
| if Config.job_config.is_development: | ||
| cls._check_and_execute_jobs_if_development_mode() | ||
| else: | ||
| if wait: | ||
| cls.__wait_until_job_finished(jobs, timeout=timeout) | ||
| elif wait: | ||
| cls.__wait_until_job_finished(jobs, timeout=timeout) |
There was a problem hiding this comment.
Function _Orchestrator.submit refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend) - Merge else clause's nested if statement into elif (
merge-else-if-into-elif)
| else: | ||
| if wait: | ||
| cls.__wait_until_job_finished(job, timeout=timeout) | ||
| elif wait: | ||
| cls.__wait_until_job_finished(job, timeout=timeout) |
There was a problem hiding this comment.
Function _Orchestrator.submit_task refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif)
| job = _JobManagerFactory._build_manager()._create( | ||
| task, itertools.chain([cls._on_status_change], callbacks or []), submit_id, submit_entity_id, force=force | ||
| return _JobManagerFactory._build_manager()._create( | ||
| task, | ||
| itertools.chain([cls._on_status_change], callbacks or []), | ||
| submit_id, | ||
| submit_entity_id, | ||
| force=force, | ||
| ) | ||
|
|
||
| return job |
There was a problem hiding this comment.
Function _Orchestrator._lock_dn_output_and_create_job refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
| if timeout: | ||
| return (datetime.now() - start).seconds < timeout | ||
| return True | ||
| return (datetime.now() - start).seconds < timeout if timeout else True |
There was a problem hiding this comment.
Function _Orchestrator.__wait_until_job_finished 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) - Replace assignment with augmented assignment (
aug-assign)
| to_cancel_or_abandon_jobs = set([job]) | ||
| to_cancel_or_abandon_jobs = {job} |
There was a problem hiding this comment.
Function _Orchestrator.cancel_job refactored with the following changes:
- Unwrap a constant iterable constructor (
unwrap-iterable-construction)
| query = query + f" AND {table_name}.version IN ({','.join(['?']*len(versions))})" | ||
| query += f" AND {table_name}.version IN ({','.join(['?'] * len(versions))})" |
There was a problem hiding this comment.
Function _SQLRepository.__get_entities_by_config_and_owner refactored with the following changes:
- Replace assignment with augmented assignment (
aug-assign)
| d = {} | ||
| for idx, col in enumerate(cursor.description): | ||
| d[col[0]] = row[idx] | ||
| return d | ||
| return {col[0]: row[idx] for idx, col in enumerate(cursor.description)} |
There was a problem hiding this comment.
Function dict_factory refactored with the following changes:
- Convert for loop into dictionary comprehension (
dict-comprehension) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| migration_fct = Config.unique_sections[MigrationConfig.name].migration_fcts.get(version, {}).get(config_id) | ||
| if migration_fct: | ||
| if ( | ||
| migration_fct := Config.unique_sections[MigrationConfig.name] | ||
| .migration_fcts.get(version, {}) | ||
| .get(config_id) | ||
| ): |
There was a problem hiding this comment.
Function __get_migration_fcts_to_latest refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
| if force: | ||
| cls.__logger.warning( | ||
| f"Option --force is detected, overriding the configuration of version {id} ..." | ||
| ) | ||
| version.config = Config._applied_config | ||
| else: | ||
| if not force: | ||
| raise ConflictedConfigurationError() | ||
|
|
||
| cls.__logger.warning( | ||
| f"Option --force is detected, overriding the configuration of version {id} ..." | ||
| ) | ||
| version.config = Config._applied_config |
There was a problem hiding this comment.
Function _VersionManager._get_or_create refactored with the following changes:
- Swap positions of nested conditionals [×2] (
swap-nested-ifs) - Hoist nested repeated code outside conditional statements [×2] (
hoist-similar-statement-from-if) - Swap if/else branches (
swap-if-else-branches) - Remove unnecessary else after guard condition (
remove-unnecessary-else)
| def __check_production_migration_config(self): | ||
| def __check_production_migration_config(cls): | ||
| from ..config.checkers._migration_config_checker import _MigrationConfigChecker | ||
|
|
||
| collector = _MigrationConfigChecker(Config._applied_config, IssueCollector())._check() | ||
| for issue in collector._warnings: | ||
| self.__logger.warning(str(issue)) | ||
| cls.__logger.warning(str(issue)) | ||
| for issue in collector._infos: | ||
| self.__logger.info(str(issue)) | ||
| cls.__logger.info(str(issue)) | ||
| for issue in collector._errors: | ||
| self.__logger.error(str(issue)) | ||
| cls.__logger.error(str(issue)) |
There was a problem hiding this comment.
Function _VersionManager.__check_production_migration_config refactored with the following changes:
- The first argument to class methods should be
cls(class-method-first-arg-name)
Branch
developrefactored 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
developbranch, then run:Help us improve this pull request!