diff --git a/.github/workflows/ruff.yml b/.github/workflows/ruff.yml index 0195d281..8a8c709c 100644 --- a/.github/workflows/ruff.yml +++ b/.github/workflows/ruff.yml @@ -15,8 +15,8 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install ruff + pip install ruff==0.16.0 - name: Run Ruff run: | ruff check . - ruff format --check . \ No newline at end of file + ruff format --check . diff --git a/babs/base.py b/babs/base.py index 0f6ed038..5124b190 100644 --- a/babs/base.py +++ b/babs/base.py @@ -133,7 +133,7 @@ def __init__(self, project_root, container_config=None): with open(root_config_path) as f: cfg = yaml.safe_load(f) or {} if not isinstance(cfg, dict): - raise ValueError( + raise TypeError( f'container_config_yaml must be a YAML mapping (key: value pairs), ' f'got {type(cfg).__name__}' ) @@ -223,11 +223,13 @@ def _validate_pipeline_config(self) -> None: Raises ------ + TypeError + If the pipeline or one of its steps has an invalid type. ValueError - If the pipeline configuration is invalid. + If the pipeline configuration is otherwise invalid. """ if not isinstance(self.pipeline, list): - raise ValueError('Pipeline configuration must be a list of steps') + raise TypeError('Pipeline configuration must be a list of steps') if len(self.pipeline) == 0: raise ValueError('Pipeline configuration cannot be empty') @@ -236,7 +238,7 @@ def _validate_pipeline_config(self) -> None: for i, step in enumerate(self.pipeline): if not isinstance(step, dict): - raise ValueError(f'Pipeline step {i} must be a dictionary') + raise TypeError(f'Pipeline step {i} must be a dictionary') required_fields = ['container_name'] for field in required_fields: @@ -356,8 +358,8 @@ def wtf_key_info(self, flag_output_ria_only=False) -> None: 'output', ], stdout=subprocess.PIPE, + check=True, ) - proc_output_ria_data_dir.check_returncode() self.output_ria_data_dir = urlparse( proc_output_ria_data_dir.stdout.decode('utf-8') ).path.strip() @@ -376,9 +378,9 @@ def wtf_key_info(self, flag_output_ria_only=False) -> None: ['datalad', '-f', "'{infos[dataset][id]}'", 'wtf', '-S', 'dataset'], cwd=self.analysis_path, stdout=subprocess.PIPE, + check=True, ) # datalad -f '{infos[dataset][id]}' wtf -S dataset - proc_analysis_dataset_id.check_returncode() self.analysis_dataset_id = ( proc_analysis_dataset_id.stdout.decode('utf-8').strip().lstrip("'").rstrip("'") ) @@ -474,6 +476,7 @@ def ensure_shared_group_git_safe_directories(self) -> None: ['git', 'config', '--global', '--get-all', 'safe.directory'], capture_output=True, text=True, + check=False, ) existing = ( {line.strip() for line in proc_existing.stdout.splitlines() if line.strip()} @@ -488,6 +491,7 @@ def ensure_shared_group_git_safe_directories(self) -> None: ['git', 'config', '--global', '--add', 'safe.directory', repo_path], capture_output=True, text=True, + check=True, ) existing.add(repo_path) @@ -533,8 +537,7 @@ def datalad_save( # Create a temporary .gitignore file to exclude specified files gitignore_path = op.join(self.analysis_path, '.gitignore') with open(gitignore_path, 'w') as f: - for file in filter_files: - f.write(f'{file}\n') + f.writelines(f'{file}\n' for file in filter_files) try: statuses = self.analysis_datalad_handle.save(path=path, message=message) @@ -552,7 +555,7 @@ def datalad_save( if not saved_status.issubset({'ok', 'notneeded'}): # exists element in `saved_status` that is not "ok" or "notneeded" # ^^ "notneeded": nothing to save - raise Exception('`datalad save` failed!') + raise RuntimeError('`datalad save` failed!') def _get_results_branches(self) -> list[str]: """Get the results branch names from the output RIA in a list.""" diff --git a/babs/bootstrap.py b/babs/bootstrap.py index 8f4102da..06550268 100644 --- a/babs/bootstrap.py +++ b/babs/bootstrap.py @@ -150,29 +150,26 @@ def babs_bootstrap( # if exists already, remove it: if op.exists(gitignore_path): os.remove(gitignore_path) - gitignore_file = open(gitignore_path, 'a') # open in append mode - - # not to track input/output RIA stores: - gitignore_file.write('\n' + op.basename(self.input_ria_path)) - gitignore_file.write('\n' + op.basename(self.output_ria_path)) - # not to track `logs` folder: - if 'logs' not in (no_ignore or []): - gitignore_file.write('\nlogs') - # not to track `.*_datalad_lock`: - gitignore_file.write('\n.*_datalad_lock') - # not to track lock file: - gitignore_file.write('\n' + 'code/babs_proj_config.yaml.lock') - # not to track `job_status.csv`: - gitignore_file.write('\n' + 'code/job_status.csv') - gitignore_file.write('\n' + 'code/job_status.csv.lock') - gitignore_file.write('\n' + 'code/job_submit.csv') - gitignore_file.write('\n' + 'code/job_submit.csv.lock') - # not to track files generated by `babs check-setup`: - gitignore_file.write('\n' + 'code/check_setup/test_job_info.yaml') - gitignore_file.write('\n' + 'code/check_setup/check_env.yaml') - gitignore_file.write('\n') - - gitignore_file.close() + with open(gitignore_path, 'a') as gitignore_file: # open in append mode + # not to track input/output RIA stores: + gitignore_file.write('\n' + op.basename(self.input_ria_path)) + gitignore_file.write('\n' + op.basename(self.output_ria_path)) + # not to track `logs` folder: + if 'logs' not in (no_ignore or []): + gitignore_file.write('\nlogs') + # not to track `.*_datalad_lock`: + gitignore_file.write('\n.*_datalad_lock') + # not to track lock file: + gitignore_file.write('\n' + 'code/babs_proj_config.yaml.lock') + # not to track `job_status.csv`: + gitignore_file.write('\n' + 'code/job_status.csv') + gitignore_file.write('\n' + 'code/job_status.csv.lock') + gitignore_file.write('\n' + 'code/job_submit.csv') + gitignore_file.write('\n' + 'code/job_submit.csv.lock') + # not to track files generated by `babs check-setup`: + gitignore_file.write('\n' + 'code/check_setup/test_job_info.yaml') + gitignore_file.write('\n' + 'code/check_setup/check_env.yaml') + gitignore_file.write('\n') self.datalad_save(path='.gitignore', message='Save .gitignore file') # Create `babs_proj_config.yaml` file: ---------------------- @@ -246,13 +243,12 @@ def babs_bootstrap( commit_message = f"Register input data dataset '{dataset_name}' as a subdataset" git_cmd = ['git', 'commit', '--amend', '-m', commit_message] - result = subprocess.run( + subprocess.run( git_cmd, cwd=self.analysis_path, stdout=subprocess.PIPE, check=True, ) - result.check_returncode() # Perform checks on the inputs: self.input_datasets.validate_input_contents() @@ -636,12 +632,12 @@ def clean_up(self): # `git annex dead here`: print('\nRunning `git annex dead here`...') - proc_git_annex_dead = subprocess.run( + subprocess.run( ['git', 'annex', 'dead', 'here'], cwd=self.analysis_path, stdout=subprocess.PIPE, + check=True, ) - proc_git_annex_dead.check_returncode() # Update input and output RIA: print('\nUpdating input and output RIA if created...') @@ -654,10 +650,7 @@ def clean_up(self): # Now we can delete this project folder: print('\nDeleting created BABS project folder...') - proc_rm_project_folder = subprocess.run( - ['rm', '-rf', self.project_root], stdout=subprocess.PIPE - ) - proc_rm_project_folder.check_returncode() + subprocess.run(['rm', '-rf', self.project_root], stdout=subprocess.PIPE, check=True) # confirm the BABS project has been removed: assert not op.exists(self.project_root), ( diff --git a/babs/check_setup.py b/babs/check_setup.py index 0249fb78..1b33924e 100644 --- a/babs/check_setup.py +++ b/babs/check_setup.py @@ -67,7 +67,7 @@ def babs_check_setup(self, submit_a_test_job): } # statuses should be all "clean", without anything else e.g., "modified": - if not analysis_statuses == {'clean'}: + if analysis_statuses != {'clean'}: problem_statuses = [ status for status in self.analysis_datalad_handle.status(eval_subdataset_state='commit') @@ -177,9 +177,8 @@ def babs_check_setup(self, submit_a_test_job): + " in 'analysis/code' folder in this BABS project!" ) # check if bash files are executable: - if op.splitext(temp_fn)[1] == '.sh': # extension is '.sh': - if not os.access(temp_fn, os.X_OK): - raise PermissionError('This code file should be executable: ' + temp_fn) + if op.splitext(temp_fn)[1] == '.sh' and not os.access(temp_fn, os.X_OK): + raise PermissionError('This code file should be executable: ' + temp_fn) print(CHECK_MARK + ' All good!') # Check input and output RIA: ---------------------- @@ -209,7 +208,7 @@ def babs_check_setup(self, submit_a_test_job): analysis_siblings = self.analysis_datalad_handle.siblings(action='query') has_sibling_input = False has_sibling_output = False - for i_sibling in range(0, len(analysis_siblings)): + for i_sibling in range(len(analysis_siblings)): the_sibling = analysis_siblings[i_sibling] if the_sibling['name'] == 'output': # output ria: has_sibling_output = True @@ -299,7 +298,7 @@ def _submit_test_job(self): new_job_status = request_all_job_status(self.queue, job_id) if not job_status.shape[0] == 1: - raise Exception( + raise RuntimeError( f'Expected 1 job for {step_container_name}, got {job_status.shape[0]}' ) @@ -320,14 +319,14 @@ def _submit_test_job(self): if op.exists(fn_check_env_yaml): flag_writable, flag_all_installed = print_versions_from_yaml(fn_check_env_yaml) if not flag_writable: - raise Exception( + raise RuntimeError( f'The designated workspace is not writable for {step_container_name}!' ' Please change it in the YAML file' ' used in `babs init --container-config`,' ' then rerun `babs init` with updated YAML file.' ) if not flag_all_installed: - raise Exception( + raise RuntimeError( f'Some required package(s) were not installed for ' f'{step_container_name} in the designated environment!' ' Please install them in the designated environment,' @@ -349,7 +348,7 @@ def _submit_test_job(self): new_job_status = request_all_job_status(self.queue, job_id) if not job_status.shape[0] == 1: - raise Exception(f'Expected 1 job, got {job_status.shape[0]}') + raise RuntimeError(f'Expected 1 job, got {job_status.shape[0]}') test_info = job_status.iloc[0].to_dict() @@ -366,7 +365,7 @@ def _submit_test_job(self): fn_check_env_yaml = op.join(self.analysis_path, 'code/check_setup', 'check_env.yaml') flag_writable, flag_all_installed = print_versions_from_yaml(fn_check_env_yaml) if not flag_writable: - raise Exception( + raise RuntimeError( 'The designated workspace is not writable!' ' Please change it in the YAML file' ' used in `babs init --container-config`,' @@ -375,7 +374,7 @@ def _submit_test_job(self): # NOTE: ^^ currently this is not aligned with YAML file sections; # this will make more sense after adding section of workspace path in YAML file if not flag_all_installed: - raise Exception( + raise RuntimeError( 'Some required package(s) were not installed' ' in the designated environment!' ' Please install them in the designated environment,' diff --git a/babs/cli.py b/babs/cli.py index b9697b95..37f54e2a 100644 --- a/babs/cli.py +++ b/babs/cli.py @@ -229,14 +229,14 @@ def babs_init_main( shared_group=shared_group, no_ignore=no_ignore, ) - except Exception as exc: + except Exception: print('\n`babs init` failed! Below is the error message:') if not keep_if_failed: print('\nCleaning up created BABS project...') babs_proj.clean_up() else: print('\n`--keep-if-failed` is requested, so not to clean up created BABS project.') - raise exc + raise def _parse_check_setup(): diff --git a/babs/container.py b/babs/container.py index 8eebfc76..76b6211d 100644 --- a/babs/container.py +++ b/babs/container.py @@ -227,9 +227,11 @@ def generate_bash_test_job(self, folder_check_setup, system, shared_group_mode=F os.chmod(fn_call_test_job, 0o770 if shared_group_mode else 0o700) # Copy the template file into the check_setup folder - with resources.files('babs').joinpath('template_test_job.py').open('rb') as src: - with open(fn_test_job, 'wb') as dst: - dst.write(src.read()) + with ( + resources.files('babs').joinpath('template_test_job.py').open('rb') as src, + open(fn_test_job, 'wb') as dst, + ): + dst.write(src.read()) os.chmod(fn_test_job, 0o770 if shared_group_mode else 0o700) def generate_job_submit_template(self, yaml_path, babs, system, test=False): diff --git a/babs/input_dataset.py b/babs/input_dataset.py index 45aba31d..1c9867b8 100644 --- a/babs/input_dataset.py +++ b/babs/input_dataset.py @@ -104,7 +104,7 @@ def is_up_to_date(self): origin_ds = dlapi.Dataset(self.origin_url) origin_sha = origin_ds.repo.get_hexsha() - if not babs_sha == origin_sha: + if babs_sha != origin_sha: print(f'Input dataset {self.name} is not up to date.') print(f'BABS SHA: {babs_sha}') print(f'Origin SHA: {origin_sha}') @@ -287,7 +287,7 @@ def as_dict(self): def validate_zipped_input_contents( dataset_abs_path, root_dir_name, processing_level, included_subjects_df=None ): - """ """ + """Validate the contents of a zipped input dataset.""" zip_pattern = ( f'sub-*_ses-*_{root_dir_name}*.zip' if processing_level == 'session' diff --git a/babs/interaction.py b/babs/interaction.py index 5dc98131..6f6ddc3b 100644 --- a/babs/interaction.py +++ b/babs/interaction.py @@ -92,7 +92,7 @@ def babs_submit(self, count=None, submit_df=None, skip_failed=False, skip_runnin ) if non_cg_states.any(): if not skip_running_jobs: - raise Exception( + raise RuntimeError( 'There are still jobs running. ' 'Please wait for them to finish or cancel them. ' 'Current running jobs:\n' diff --git a/babs/merge.py b/babs/merge.py index 9b41e7d3..b9c2506c 100644 --- a/babs/merge.py +++ b/babs/merge.py @@ -46,6 +46,7 @@ def robust_rm_dir(path, max_retries=3, retry_delay=1): cwd=path, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) dlapi.remove(path=path, dataset=path, reckless='availability') # datalad remove might not remove everything, check if path still exists @@ -121,7 +122,7 @@ def babs_merge(self, chunk_size=1000, trial_run=False): merge_ds_path = op.join(self.project_root, 'merge_ds') if op.exists(merge_ds_path): - raise Exception( + raise RuntimeError( "Folder 'merge_ds' already exists. `babs merge` won't proceed." " If you're sure you want to rerun `babs merge`," ' please remove this folder before you rerun `babs merge`.' @@ -165,8 +166,8 @@ def babs_merge(self, chunk_size=1000, trial_run=False): ['git', 'remote', 'show', 'origin'], cwd=merge_ds_path, stdout=subprocess.PIPE, + check=True, ) - proc_git_remote_show_origin.check_returncode() msg = proc_git_remote_show_origin.stdout.decode('utf-8') # e.g., '... HEAD branch: master\n....': search between 'HEAD branch: ' and '\n': temp = re.search('HEAD branch: ' + '(.+?)' + '\n', msg) @@ -174,7 +175,7 @@ def babs_merge(self, chunk_size=1000, trial_run=False): default_branch_name = temp.group(1) # what's between those two keywords # another way: `default_branch_name = msg.split("HEAD branch: ")[1].split("\n")[0]` else: - raise Exception('There is no HEAD branch in output RIA!') + raise RuntimeError('There is no HEAD branch in output RIA!') print("Git default branch's name of output RIA is: '" + default_branch_name + "'") # get current git commit SHASUM before merging as a reference: @@ -195,7 +196,7 @@ def babs_merge(self, chunk_size=1000, trial_run=False): # check if there is any valid job (with results): if len(list_branches_with_results) == 0: # empty: - raise Exception( + raise RuntimeError( 'There is no job branch in output RIA that has results yet,' ' i.e., there is no successfully finished job yet.' ' Please run `babs submit` first.' @@ -242,7 +243,7 @@ def babs_merge(self, chunk_size=1000, trial_run=False): # ^^ e.g., [['1', '7', '0'], ['6', '2'], ['5', '6']] # iterate across chunks: - for i_chunk in range(0, num_chunks): + for i_chunk in range(num_chunks): print( 'Merging chunk #' + str(i_chunk + 1) @@ -261,7 +262,9 @@ def babs_merge(self, chunk_size=1000, trial_run=False): # Prepend 'origin/' to each branch name remote_branches = ['origin/' + branch for branch in joined_by_space.split(' ')] cmd = ['git', 'merge', '-m', commit_msg] + remote_branches - proc_git_merge = subprocess.run(cmd, cwd=merge_ds_path, capture_output=True, text=True) + proc_git_merge = subprocess.run( + cmd, cwd=merge_ds_path, capture_output=True, text=True, check=False + ) if proc_git_merge.returncode != 0: print(f'Git merge failed with error:\n{proc_git_merge.stderr}') proc_git_merge.check_returncode() @@ -269,7 +272,7 @@ def babs_merge(self, chunk_size=1000, trial_run=False): # Push merging actions back to output RIA: if trial_run: - print('') # new empty line + print() # new empty line warnings.warn( '`--trial-run` was requested, not to push merging actions to output RIA.', stacklevel=2, @@ -279,8 +282,9 @@ def babs_merge(self, chunk_size=1000, trial_run=False): print('\nPushing merging actions to output RIA...') # `git push`: - proc_git_push = subprocess.run(['git', 'push'], cwd=merge_ds_path, stdout=subprocess.PIPE) - proc_git_push.check_returncode() + proc_git_push = subprocess.run( + ['git', 'push'], cwd=merge_ds_path, stdout=subprocess.PIPE, check=True + ) print(proc_git_push.stdout.decode('utf-8')) # Get file availability information: which is very important! @@ -293,8 +297,8 @@ def babs_merge(self, chunk_size=1000, trial_run=False): ['git', 'annex', 'fsck', '--fast', '-f', 'output-storage'], cwd=merge_ds_path, stdout=subprocess.PIPE, + check=True, ) - proc_git_annex_fsck.check_returncode() # if printing the returned msg, # will be a long list of "fsck xxx.zip (fixing location log) ok" # or "fsck xxx.zip ok" @@ -315,8 +319,8 @@ def babs_merge(self, chunk_size=1000, trial_run=False): ['git', 'annex', 'find', '--not', '--in', 'output-storage'], cwd=merge_ds_path, stdout=subprocess.PIPE, + check=True, ) - proc_git_annex_find_missing.check_returncode() msg = proc_git_annex_find_missing.stdout.decode('utf-8') # `msg` should be empty: if msg != '': # if not empty: @@ -324,7 +328,7 @@ def babs_merge(self, chunk_size=1000, trial_run=False): with open(fn_list_content_missing, 'w') as f: f.write(msg) f.write('\n') - raise Exception( + raise RuntimeError( 'Unable to find file content for some file(s).' " The information has been saved to this text file: '" + fn_list_content_missing @@ -338,8 +342,8 @@ def babs_merge(self, chunk_size=1000, trial_run=False): ['git', 'annex', 'dead', 'here'], cwd=merge_ds_path, stdout=subprocess.PIPE, + check=True, ) - proc_git_annex_dead_here.check_returncode() print(proc_git_annex_dead_here.stdout.decode('utf-8')) # Final `datalad push` to output RIA: @@ -351,8 +355,8 @@ def babs_merge(self, chunk_size=1000, trial_run=False): ['datalad', 'push', '--data', 'nothing'], cwd=merge_ds_path, stdout=subprocess.PIPE, + check=True, ) - proc_datalad_push.check_returncode() print(proc_datalad_push.stdout.decode('utf-8')) # Done: @@ -375,6 +379,6 @@ def babs_merge(self, chunk_size=1000, trial_run=False): ['git', 'branch', '--delete'] + chunk, cwd=self.output_ria_data_dir, stdout=subprocess.PIPE, + check=True, ) - proc_git_branch_delete.check_returncode() print(proc_git_branch_delete.stdout.decode('utf-8')) diff --git a/babs/scheduler.py b/babs/scheduler.py index f0097d0d..a44881bd 100644 --- a/babs/scheduler.py +++ b/babs/scheduler.py @@ -42,7 +42,7 @@ def run_squeue(queue, job_id: int) -> str: f'-j{job_id}', ] - result = subprocess.run(cmd, capture_output=True, text=True) + result = subprocess.run(cmd, capture_output=True, text=True, check=False) if result.returncode == 1 and 'Invalid job id specified' in result.stderr: return '' @@ -130,6 +130,7 @@ def squeue_to_pandas(job_id=None) -> pd.DataFrame: commandlist, capture_output=True, text=True, + check=False, ) # Check if command failed @@ -160,7 +161,7 @@ def squeue_to_pandas(job_id=None) -> pd.DataFrame: skipinitialspace=True, ) except Exception as e: - raise RuntimeError(f'Failed to parse squeue output: {str(e)}\nOutput was: {result.stdout}') + raise RuntimeError(f'Failed to parse squeue output: {e!s}\nOutput was: {result.stdout}') # separate job_id into job_id and task_id df['task_id'] = df['job_id'].str.split('_').str[1].astype(int) @@ -205,6 +206,7 @@ def sbatch_get_job_id(sbatch_cmd_list, working_dir): cwd=working_dir, capture_output=True, text=True, + check=False, ) if proc_cmd.returncode != 0: raise RuntimeError(f'Failed to submit array job: {proc_cmd.stderr}') @@ -257,7 +259,7 @@ def submit_array(analysis_path, queue, maxarray): if queue == 'slurm': job_id = sbatch_get_job_id(cmd.split(), analysis_path) else: - raise Exception('Invalid job scheduler system type `queue`: ' + queue) + raise ValueError('Invalid job scheduler system type `queue`: ' + queue) return job_id @@ -302,7 +304,7 @@ def submit_one_test_job(analysis_path, queue): if queue == 'slurm': job_id = sbatch_get_job_id(cmd.split(), analysis_path) else: - raise Exception('Invalid job scheduler system type `queue`: ' + queue) + raise ValueError('Invalid job scheduler system type `queue`: ' + queue) print(f'Test job has been submitted (job ID: {job_id}).') return job_id diff --git a/babs/system.py b/babs/system.py index 92a363b4..9ece0736 100644 --- a/babs/system.py +++ b/babs/system.py @@ -63,7 +63,7 @@ def get_dict(self): # sanity check: if self.type not in dict: - raise Exception( + raise ValueError( "There is no key called '" + self.type + "' in" diff --git a/babs/template_test_job.py b/babs/template_test_job.py index 87b8a93e..c2959cd1 100644 --- a/babs/template_test_job.py +++ b/babs/template_test_job.py @@ -34,52 +34,47 @@ def main(): fn_yaml = op.join(args.path_check_setup, 'check_env.yaml') if op.exists(fn_yaml): os.remove(fn_yaml) # remove it - yaml_file = open(fn_yaml, 'w') + with open(fn_yaml, 'w') as yaml_file: + # Initialize the dict: + config = {} - # Initialize the dict: - config = {} + # If the path of ephemeral compute workspace is writable: + flag_writable = os.access(args.path_workspace, os.W_OK) + config['workspace_writable'] = flag_writable + # change to the version that `read_yaml()` from babs/utils.py can read: + if flag_writable: # True + str_writable = 'true' + else: + str_writable = 'false' + yaml_file.write('workspace_writable: ' + str_writable + '\n') - # If the path of ephemeral compute workspace is writable: - flag_writable = os.access(args.path_workspace, os.W_OK) - config['workspace_writable'] = flag_writable - # change to the version that `read_yaml()` from babs/utils.py can read: - if flag_writable: # True - str_writable = 'true' - else: - str_writable = 'false' - yaml_file.write('workspace_writable: ' + str_writable + '\n') + # Which python in current env: + # assume the python is installed; otherwise this script cannot be run: + config['which_python'] = sys.executable + yaml_file.write("which_python: '" + sys.executable + "'\n") - # Which python in current env: - # assume the python is installed; otherwise this script cannot be run: - config['which_python'] = sys.executable - yaml_file.write("which_python: '" + sys.executable + "'\n") + # Check each dependent packages' versions: + config['version'] = {} + yaml_file.write('version:\n') + # What packages' versions to check: + what_versions = { + 'datalad': 'datalad --version', + 'git': 'git --version', + 'git-annex': 'git-annex version', + 'datalad_containers': 'datalad containers-add --version', + } + for key, the_command in what_versions.items(): + try: + proc = subprocess.run(the_command.split(' '), stdout=subprocess.PIPE, check=True) + if key == 'git-annex': + temp = proc.stdout.decode('utf-8').split('\n')[0] + config['version'][key] = temp + else: + config['version'][key] = proc.stdout.decode('utf-8').replace('\n', '') + except Exception: + config['version'][key] = 'not_installed' - # Check each dependent packages' versions: - config['version'] = {} - yaml_file.write('version:\n') - # What packages' versions to check: - what_versions = { - 'datalad': 'datalad --version', - 'git': 'git --version', - 'git-annex': 'git-annex version', - 'datalad_containers': 'datalad containers-add --version', - } - for key in what_versions: - the_command = what_versions[key] - try: - proc = subprocess.run(the_command.split(' '), stdout=subprocess.PIPE) - proc.check_returncode() - if key == 'git-annex': - temp = proc.stdout.decode('utf-8').split('\n')[0] - config['version'][key] = temp - else: - config['version'][key] = proc.stdout.decode('utf-8').replace('\n', '') - except Exception: - config['version'][key] = 'not_installed' - - yaml_file.write(' ' + key + ": '" + config['version'][key] + "'\n") - - yaml_file.close() + yaml_file.write(' ' + key + ": '" + config['version'][key] + "'\n") if __name__ == '__main__': diff --git a/babs/utils.py b/babs/utils.py index 06fb7a6f..f3f47b66 100644 --- a/babs/utils.py +++ b/babs/utils.py @@ -118,10 +118,9 @@ def read_yaml(fn, use_filelock=False): lock = FileLock(lock_path) try: - with lock.acquire(timeout=5): # lock the file, i.e., lock job status df - with open(fn) as f: - config = yaml.safe_load(f) - # ^^ dict is a dict; elements can be accessed by `dict["key"]["sub-key"]` + with lock.acquire(timeout=5), open(fn) as f: + config = yaml.safe_load(f) + # ^^ dict is a dict; elements can be accessed by `dict["key"]["sub-key"]` except Timeout: # after waiting for time defined in `timeout`: # if another instance also uses locks, and is currently running, # there will be a timeout error @@ -208,7 +207,7 @@ def app_output_settings_from_config(config): # Sanity check: this section should exist: if 'zip_foldernames' not in config: - raise Exception( + raise ValueError( 'The `container_config` does not contain' ' the section `zip_foldernames`. Please add this section!' ) @@ -231,21 +230,23 @@ def app_output_settings_from_config(config): ) # Only raise an exception if both are defined and they don't match - if None not in (deprecated_create_output_dir_for_single_zip, create_output_dir_for_single_zip): - if not deprecated_create_output_dir_for_single_zip == create_output_dir_for_single_zip: - raise ValueError( - 'The `all_results_in_one_zip` and the deprecated placeholder' - "'" + PLACEHOLDER_MK_SUB_OUTPUT_FOLDER_DEPRECATED + "' do not match." - ) + if ( + None not in (deprecated_create_output_dir_for_single_zip, create_output_dir_for_single_zip) + and deprecated_create_output_dir_for_single_zip != create_output_dir_for_single_zip + ): + raise ValueError( + 'The `all_results_in_one_zip` and the deprecated placeholder' + "'" + PLACEHOLDER_MK_SUB_OUTPUT_FOLDER_DEPRECATED + "' do not match." + ) # Make sure it's not empty after we popped the deprecated key if not config['zip_foldernames']: - raise Exception('No output folder name provided in `zip_foldernames` section.') + raise ValueError('No output folder name provided in `zip_foldernames` section.') # Get the dict of foldernames + version number: if create_output_dir_for_single_zip: - if not len(config['zip_foldernames']) == 1: - raise Exception( + if len(config['zip_foldernames']) != 1: + raise ValueError( 'You ask BABS to create more than one output folder,' ' but BABS can only create one output folder.' " Please only keep one of them in 'zip_foldernames' section." @@ -290,10 +291,9 @@ def print_versions_from_yaml(fn_yaml): config = read_yaml(fn_yaml) print('Below is the information of designated environment and temporary workspace:\n') # print the yaml file: - f = open(fn_yaml) - file_contents = f.read() + with open(fn_yaml) as f: + file_contents = f.read() print(file_contents) - f.close() # Check if everything is as satisfied: if config['workspace_writable']: # bool; if writable: @@ -338,9 +338,8 @@ def get_git_show_ref_shasum(branch_name, the_path): """ proc_git_show_ref = subprocess.run( - ['git', 'show-ref', branch_name], cwd=the_path, stdout=subprocess.PIPE + ['git', 'show-ref', branch_name], cwd=the_path, stdout=subprocess.PIPE, check=True ) - proc_git_show_ref.check_returncode() msg = proc_git_show_ref.stdout.decode('utf-8') # `msg.split()`: # split by space and '\n' # e.g. for default branch (main or master): @@ -370,6 +369,7 @@ def get_results_branches(ria_directory): cwd=ria_directory, capture_output=True, text=True, + check=True, ) # Filter to just branches starting with 'job-' @@ -406,8 +406,8 @@ def get_results_branches_from_clone(clone_path): cwd=clone_path, capture_output=True, text=True, + check=True, ) - out.check_returncode() branches = [] for line in (out.stdout or '').strip().splitlines(): line = line.strip() @@ -441,6 +441,7 @@ def get_results_branches_from_ria(ria_data_dir, timeout=30): capture_output=True, text=True, timeout=timeout, + check=False, ) if out.returncode != 0: return [] @@ -528,7 +529,11 @@ def get_repo_hash(repo_path): the hash of the current commit """ proc_hash = subprocess.run( - ['git', 'rev-parse', 'HEAD'], cwd=repo_path, capture_output=True, text=True + ['git', 'rev-parse', 'HEAD'], + cwd=repo_path, + capture_output=True, + text=True, + check=False, ) if proc_hash.returncode != 0: raise ValueError( @@ -658,34 +663,34 @@ def validate_sub_ses_processing_inclusion(processing_inclusion_file, processing_ try: initial_inclu_df = pd.read_csv(processing_inclusion_file) except Exception as e: - raise Exception(f'Error reading `{processing_inclusion_file}`:\n{e}') + raise ValueError(f'Error reading `{processing_inclusion_file}`:\n{e}') # Sanity check: there are expected column(s): if 'sub_id' not in initial_inclu_df.columns: - raise Exception( + raise ValueError( f'Error reading `{processing_inclusion_file}`: ' f"There is no 'sub_id' column in the CSV file!" ) if processing_level == 'session' and 'ses_id' not in initial_inclu_df.columns: - raise Exception( + raise ValueError( "There is no 'ses_id' column in `processing_inclusion_file`! " 'It is expected as user requested to process data on a session-wise basis.' ) # Sanity check: no repeated sub (or sessions): - if processing_level == 'subject': + if processing_level == 'subject' and initial_inclu_df['sub_id'].duplicated().any(): # there should only be one occurrence per sub: - if initial_inclu_df['sub_id'].duplicated().any(): - raise Exception("There are repeated 'sub_id' in `processing_inclusion_file`!") - - elif processing_level == 'session': + raise ValueError("There are repeated 'sub_id' in `processing_inclusion_file`!") + elif ( + processing_level == 'session' + and initial_inclu_df.duplicated(subset=['sub_id', 'ses_id']).any() + ): # there should not be repeated combinations of `sub_id` and `ses_id`: - if initial_inclu_df.duplicated(subset=['sub_id', 'ses_id']).any(): - raise Exception( - "There are repeated combinations of 'sub_id' and 'ses_id' in " - f'`{processing_inclusion_file}`!' - ) + raise ValueError( + "There are repeated combinations of 'sub_id' and 'ses_id' in " + f'`{processing_inclusion_file}`!' + ) # Sort the initial included sub/ses list: sorting_indices = ['sub_id'] if processing_level == 'subject' else ['sub_id', 'ses_id'] initial_inclu_df = initial_inclu_df.sort_values(by=sorting_indices).reset_index(drop=True) diff --git a/design/status-without-pandas.md b/design/status-without-pandas.md index 4ac63b39..0626424a 100644 --- a/design/status-without-pandas.md +++ b/design/status-without-pandas.md @@ -99,14 +99,15 @@ total_running, total_failed. ```python class SchedulerState(Enum): - NOT_SUBMITTED = "NOT_SUBMITTED" - PENDING = "PD" - RUNNING = "R" - COMPLETING = "CG" - CONFIGURING = "CF" - DONE = "DONE" # left scheduler, exit code unknown + NOT_SUBMITTED = 'NOT_SUBMITTED' + PENDING = 'PD' + RUNNING = 'R' + COMPLETING = 'CG' + CONFIGURING = 'CF' + DONE = 'DONE' # left scheduler, exit code unknown # future: COMPLETED, FAILED, CANCELLED, TIMEOUT (from sacct) + @dataclass class JobStatus: sub_id: str @@ -124,8 +125,7 @@ class JobStatus: @property def is_failed(self) -> bool: - return (self.scheduler_state == SchedulerState.DONE - and not self.has_results) + return self.scheduler_state == SchedulerState.DONE and not self.has_results @property def submitted(self) -> bool: diff --git a/pyproject.toml b/pyproject.toml index 51a59da3..bde710fc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -46,7 +46,7 @@ doc = [ "sphinx_design", # for adding in-line badges etc "sphinx_rtd_theme", # needed by readthedocs ] -dev = ["ruff ~= 0.4.3", "pre-commit"] +dev = ["ruff == 0.16.0", "pre-commit"] tests = [ "coverage", "pytest", diff --git a/tests/conftest.py b/tests/conftest.py index 4c4cfd6d..af752d5d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,7 @@ def _setup_before_all_tests(): ['sacctmgr', '-i', 'modify', 'user', 'root', 'set', 'MaxJobs=200'], capture_output=True, text=True, + check=False, ) if result.returncode != 0: raise RuntimeError( @@ -54,7 +55,7 @@ def simbids_apptainer_image(): Get the path of the simbids-raw-mri apptainer image. """ if not op.exists(f'/singularity_images/simbids_{SIMBIDS_VERSION}.sif'): - raise Exception(f'simbids_{SIMBIDS_VERSION}.sif not found!') + raise FileNotFoundError(f'simbids_{SIMBIDS_VERSION}.sif not found!') return f'/singularity_images/simbids_{SIMBIDS_VERSION}.sif' @@ -91,7 +92,7 @@ def get_simbids_raw_bids_data(simbids_apptainer_image_path, bids_dir, session_ty 'ds004146_configs.yaml' if session_type == 'multi-session' else 'ds005237_configs.yaml' ) - proc = subprocess.run( + subprocess.run( [ 'apptainer', 'exec', @@ -103,8 +104,8 @@ def get_simbids_raw_bids_data(simbids_apptainer_image_path, bids_dir, session_ty simbids_yaml, ], stdout=subprocess.PIPE, + check=True, ) - proc.check_returncode() # Initialize datalad in the bids_dir, forcing it to accept original files ds_path = str(bids_dir.absolute() / 'simbids') assert op.exists(ds_path) @@ -160,8 +161,7 @@ def run_simbids_app_simulation( ] if extra_args: args.extend(extra_args) - proc = subprocess.run(args, stdout=subprocess.PIPE) - proc.check_returncode() + subprocess.run(args, stdout=subprocess.PIPE, check=True) return app_output_dir @@ -362,6 +362,7 @@ def gather_slurm_job_diagnostics( capture_output=True, text=True, timeout=10, + check=False, ) lines.append(f'sacct stdout:\n{out.stdout or "(none)"}') if out.stderr: diff --git a/tests/test_babs_workflow.py b/tests/test_babs_workflow.py index 4e2707a9..d965aa30 100644 --- a/tests/test_babs_workflow.py +++ b/tests/test_babs_workflow.py @@ -82,16 +82,20 @@ def test_babs_init_raw_bids( # Test error when project root already exists project_root.mkdir(parents=True, exist_ok=True) - with mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=babs_init_opts): - with pytest.raises(FileExistsError, match=r'already exists'): - _enter_init() + with ( + mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=babs_init_opts), + pytest.raises(FileExistsError, match=r'already exists'), + ): + _enter_init() # Test error when parent directory doesn't exist non_existent_parent = project_base / 'non_existent' / 'my_babs_project' babs_init_opts.project_root = non_existent_parent - with mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=babs_init_opts): - with pytest.raises(ValueError, match=r'parent folder.*does not exist'): - _enter_init() + with ( + mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=babs_init_opts), + pytest.raises(ValueError, match=r'parent folder.*does not exist'), + ): + _enter_init() # Test error when parent directory doesn't exist babs_init_opts.project_root = project_root @@ -231,19 +235,21 @@ def _get_results_branches_use_merge_ds_when_exists(self): return get_results_branches_from_clone(str(merge_ds)) return _orig_get_results_branches_method(self) - with mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=babs_merge_opts): - with mock.patch.object( + with ( + mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=babs_merge_opts), + mock.patch.object( babs_base.BABS, '_get_results_branches', _get_results_branches_use_merge_ds_when_exists - ): - try: - _enter_merge() - except ValueError as e: - if 'no successfully finished job' in str(e).lower(): - diag = gather_slurm_job_diagnostics( - project_root, log_glob='sim.*', max_logs=None, tail_lines=None - ) - raise ValueError(f'{e}\nJob accounting (sacct):\n{diag}') from e - raise + ), + ): + try: + _enter_merge() + except ValueError as e: + if 'no successfully finished job' in str(e).lower(): + diag = gather_slurm_job_diagnostics( + project_root, log_glob='sim.*', max_logs=None, tail_lines=None + ) + raise ValueError(f'{e}\nJob accounting (sacct):\n{diag}') from e + raise def test_init_forwards_shared_group(tmp_path): @@ -261,9 +267,11 @@ def test_init_forwards_shared_group(tmp_path): shared_group='my-lab-group', no_ignore=[], ) - with mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=options): - with mock.patch('babs.BABSBootstrap') as mock_bootstrap_cls: - _enter_init() + with ( + mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=options), + mock.patch('babs.BABSBootstrap') as mock_bootstrap_cls, + ): + _enter_init() mock_bootstrap_cls.assert_called_once_with( options.project_root, @@ -296,9 +304,11 @@ def test_init_forwards_no_ignore(tmp_path): shared_group=None, no_ignore=['logs'], ) - with mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=options): - with mock.patch('babs.BABSBootstrap') as mock_bootstrap_cls: - _enter_init() + with ( + mock.patch.object(argparse.ArgumentParser, 'parse_args', return_value=options), + mock.patch('babs.BABSBootstrap') as mock_bootstrap_cls, + ): + _enter_init() mock_bootstrap_cls.assert_called_once_with( options.project_root, @@ -468,6 +478,7 @@ def test_datalad_save_with_filtering(babs_project_sessionlevel_babsobject): cwd=babs_project_sessionlevel_babsobject.analysis_path, capture_output=True, text=True, + check=True, ) assert test_file1.name in result.stdout diff --git a/tests/test_base.py b/tests/test_base.py index 14f20bc1..53d67ffb 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -68,7 +68,7 @@ def test_validate_pipeline_config(babs_project_sessionlevel): # Test invalid configs babs_proj.pipeline = {'not': 'a list'} - with pytest.raises(ValueError, match='Pipeline configuration must be a list'): + with pytest.raises(TypeError, match='Pipeline configuration must be a list'): babs_proj._validate_pipeline_config() babs_proj.pipeline = [] @@ -76,7 +76,7 @@ def test_validate_pipeline_config(babs_project_sessionlevel): babs_proj._validate_pipeline_config() babs_proj.pipeline = ['not a dict'] - with pytest.raises(ValueError, match='Pipeline step 0 must be a dictionary'): + with pytest.raises(TypeError, match='Pipeline step 0 must be a dictionary'): babs_proj._validate_pipeline_config() babs_proj.pipeline = [{'missing': 'container_name'}] @@ -483,7 +483,7 @@ def test_shared_group_inits_analysis_and_rias( ['git', 'config', '--global', '--get-all', 'safe.directory'], stdout=subprocess.PIPE, text=True, - check=False, + check=True, ).stdout.splitlines() assert str(Path(babs_bootstrap.analysis_path).resolve()) in safe_dirs assert str(output_ria_dir.resolve()) in safe_dirs diff --git a/tests/test_check_setup.py b/tests/test_check_setup.py index 30267846..a4ffbe34 100644 --- a/tests/test_check_setup.py +++ b/tests/test_check_setup.py @@ -149,7 +149,7 @@ def mock_request_status(*args, **kwargs): sys.stdout.write('DEBUG: All captured printed messages:\n') for i, msg in enumerate(printed_messages): - sys.stdout.write(f' {i}: {repr(msg)}\n') + sys.stdout.write(f' {i}: {msg!r}\n') sys.stdout.flush() # Check expected messages and behavior diff --git a/tests/test_generate_bidsapp_runscript.py b/tests/test_generate_bidsapp_runscript.py index 19a34402..f8adee23 100644 --- a/tests/test_generate_bidsapp_runscript.py +++ b/tests/test_generate_bidsapp_runscript.py @@ -117,7 +117,7 @@ def test_generate_bidsapp_runscript(input_datasets, config_file, processing_leve config_path = NOTEBOOKS_DIR / config_file container_name = config_file.split('_')[1] config = read_yaml(config_path) - dict_zip_foldernames, bids_app_output_dir = app_output_settings_from_config(config) + _dict_zip_foldernames, bids_app_output_dir = app_output_settings_from_config(config) script_content = generate_bidsapp_runscript( input_datasets, processing_level, @@ -156,7 +156,9 @@ def run_shellcheck(script_path): try: # Run shellcheck on the temporary file - result = subprocess.run(['shellcheck', script_path], capture_output=True, text=True) + result = subprocess.run( + ['shellcheck', script_path], capture_output=True, text=True, check=False + ) return result.returncode == 0, result.stdout except subprocess.CalledProcessError as e: return False, e.output diff --git a/tests/test_generate_submit_script.py b/tests/test_generate_submit_script.py index 79ff5144..cf8034aa 100644 --- a/tests/test_generate_submit_script.py +++ b/tests/test_generate_submit_script.py @@ -138,7 +138,9 @@ def run_shellcheck(script_path): try: # Run shellcheck on the temporary file - result = subprocess.run(['shellcheck', script_path], capture_output=True, text=True) + result = subprocess.run( + ['shellcheck', script_path], capture_output=True, text=True, check=False + ) return result.returncode == 0, result.stdout except subprocess.CalledProcessError as e: return False, e.output @@ -300,7 +302,9 @@ def test_find_single_zip_handles_regex_metachars_in_name(name, processing_level, ) # the template defines the finder AND calls it, echoing the located zip path script = f'set -e\nsubid=sub-01\nsesid=ses-1\n{finder}\n' - result = subprocess.run(['bash', '-c', script], cwd=tmp_path, capture_output=True, text=True) + result = subprocess.run( + ['bash', '-c', script], cwd=tmp_path, capture_output=True, text=True, check=False + ) assert result.returncode == 0, result.stderr assert zipname in result.stdout, f'zip not located:\nOUT:{result.stdout}\nERR:{result.stderr}' diff --git a/tests/test_interaction.py b/tests/test_interaction.py index ff2f83ea..8955394c 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -111,7 +111,7 @@ def test_babs_status_configures_shared_group_runtime(babs_project_subjectlevel, lambda: called.append(True), ) # Stub downstream work; this test verifies guard invocation only. - monkeypatch.setattr(babs_proj, '_update_results_status', lambda: {}) + monkeypatch.setattr(babs_proj, '_update_results_status', dict) monkeypatch.setattr('babs.interaction.report_job_status', lambda *_args, **_kwargs: None) babs_proj.babs_status() @@ -453,9 +453,7 @@ def _make_statuses(submitted, has_results): statuses = {} for i, (sub, res) in enumerate(zip(submitted, has_results, strict=True)): sub_id = f'sub-{i + 1:02d}' - if sub and not res: - state = SchedulerState.DONE - elif sub: + if sub: state = SchedulerState.DONE else: state = SchedulerState.NOT_SUBMITTED diff --git a/tests/test_merge.py b/tests/test_merge.py index b7841a8e..52da1eca 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -16,7 +16,7 @@ def test_merge_no_branches(babs_project_sessionlevel, monkeypatch): """Test babs_merge when no branches have results.""" babs_proj = BABSMerge(babs_project_sessionlevel) - monkeypatch.setattr(babs_proj, '_get_results_branches', lambda: []) + monkeypatch.setattr(babs_proj, '_get_results_branches', list) with pytest.raises(ValueError, match='There is no successfully finished job yet'): babs_proj.babs_merge() @@ -28,16 +28,27 @@ def test_merge_all_branches_no_results(babs_project_sessionlevel, tmp_path, monk merge_ds_path = tmp_path / 'merge_ds' merge_ds_path.mkdir() - subprocess.run(['git', 'init'], cwd=merge_ds_path, capture_output=True) - subprocess.run(['git', 'config', 'user.name', 'Test'], cwd=merge_ds_path, capture_output=True) + subprocess.run(['git', 'init'], cwd=merge_ds_path, capture_output=True, check=True) + subprocess.run( + ['git', 'config', 'user.name', 'Test'], + cwd=merge_ds_path, + capture_output=True, + check=True, + ) subprocess.run( ['git', 'config', 'user.email', 'test@test.com'], cwd=merge_ds_path, capture_output=True, + check=True, ) (merge_ds_path / 'test.txt').write_text('test') - subprocess.run(['git', 'add', 'test.txt'], cwd=merge_ds_path, capture_output=True) - subprocess.run(['git', 'commit', '-m', 'Initial'], cwd=merge_ds_path, capture_output=True) + subprocess.run(['git', 'add', 'test.txt'], cwd=merge_ds_path, capture_output=True, check=True) + subprocess.run( + ['git', 'commit', '-m', 'Initial'], + cwd=merge_ds_path, + capture_output=True, + check=True, + ) default_branch = 'main' try: @@ -45,6 +56,7 @@ def test_merge_all_branches_no_results(babs_project_sessionlevel, tmp_path, monk ['git', 'checkout', '-b', default_branch], cwd=merge_ds_path, capture_output=True, + check=True, ) except Exception: default_branch = 'master' @@ -73,7 +85,7 @@ def mock_remote_show(cmd, **kwargs): result.returncode = 0 result.stdout = f'HEAD branch: {default_branch}\n'.encode() return result - return subprocess.run(cmd, **kwargs) + return subprocess.run(cmd, check=kwargs.pop('check', False), **kwargs) monkeypatch.setattr('babs.merge.subprocess.run', mock_remote_show) @@ -128,7 +140,7 @@ def test_rm_dir_datalad_fail(tmp_path, monkeypatch): # Mock datalad.remove to raise an exception def mock_remove(path=None, dataset=None, **kwargs): - raise Exception('datalad remove failed') + raise RuntimeError('datalad remove failed') monkeypatch.setattr(dlapi, 'remove', mock_remove) @@ -283,7 +295,6 @@ def set_analysis_id(): def mock_clone(source, path): # Create the directory so subsequent git commands can run os.makedirs(path, exist_ok=True) - return None monkeypatch.setattr(dlapi, 'clone', mock_clone) @@ -293,7 +304,7 @@ def mock_remote_show(cmd, **kwargs): result.returncode = 0 result.stdout = b'No HEAD branch found\n' # No HEAD branch return result - return subprocess.run(cmd, **kwargs) + return subprocess.run(cmd, check=kwargs.pop('check', False), **kwargs) monkeypatch.setattr('babs.merge.subprocess.run', mock_remote_show) diff --git a/tests/test_slurm.py b/tests/test_slurm.py index b448c17a..493ab5fa 100644 --- a/tests/test_slurm.py +++ b/tests/test_slurm.py @@ -124,7 +124,7 @@ def test_array_job_submission( test_dir = tmp_path_factory.mktemp('test_array_job') # Submit array job with 3 tasks - working_directory, job_id = submit_array_job(test_dir, array_size=3) + _working_directory, job_id = submit_array_job(test_dir, array_size=3) # Wait a moment for job to be registered import time diff --git a/tests/test_template_job.py b/tests/test_template_job.py index a72b4cf4..05efb99f 100644 --- a/tests/test_template_job.py +++ b/tests/test_template_job.py @@ -35,6 +35,7 @@ def test_template_job_execution(): capture_output=True, text=True, env={**os.environ, 'RUNNING_PYTEST': '1'}, # Mark as running in test + check=False, ) # Check that execution was successful diff --git a/tests/test_utils.py b/tests/test_utils.py index a986c8b6..4f0b0390 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -102,18 +102,31 @@ def create_git_repo(tmp_path): repo_path.mkdir() # Initialize the git repo - subprocess.run(['git', 'init'], cwd=repo_path, capture_output=True) + subprocess.run(['git', 'init'], cwd=repo_path, capture_output=True, check=True) # Configure git user name and email (required for commits) - subprocess.run(['git', 'config', 'user.name', 'Test User'], cwd=repo_path, capture_output=True) subprocess.run( - ['git', 'config', 'user.email', 'test@example.com'], cwd=repo_path, capture_output=True + ['git', 'config', 'user.name', 'Test User'], + cwd=repo_path, + capture_output=True, + check=True, + ) + subprocess.run( + ['git', 'config', 'user.email', 'test@example.com'], + cwd=repo_path, + capture_output=True, + check=True, ) # Create a test file and commit it (repo_path / 'test_file.txt').write_text('Test content') - subprocess.run(['git', 'add', 'test_file.txt'], cwd=repo_path, capture_output=True) - subprocess.run(['git', 'commit', '-m', 'Initial commit'], cwd=repo_path, capture_output=True) + subprocess.run(['git', 'add', 'test_file.txt'], cwd=repo_path, capture_output=True, check=True) + subprocess.run( + ['git', 'commit', '-m', 'Initial commit'], + cwd=repo_path, + capture_output=True, + check=True, + ) return repo_path @@ -128,7 +141,11 @@ def test_get_repo_hash(tmp_path): # Get the hash directly with git expected_hash = subprocess.run( - ['git', 'rev-parse', 'HEAD'], cwd=repo_path, capture_output=True, text=True + ['git', 'rev-parse', 'HEAD'], + cwd=repo_path, + capture_output=True, + text=True, + check=True, ).stdout.strip() # They should match @@ -149,7 +166,11 @@ def test_git_show_ref_shasum(tmp_path): # Get the current branch name branch_name = subprocess.run( - ['git', 'branch', '--show-current'], cwd=repo_path, capture_output=True, text=True + ['git', 'branch', '--show-current'], + cwd=repo_path, + capture_output=True, + text=True, + check=True, ).stdout.strip() # Get the ref with our function @@ -185,6 +206,7 @@ def test_get_results_branches_from_clone(tmp_path): cwd=str(tmp_path), capture_output=True, text=True, + check=True, ) @@ -407,20 +429,36 @@ def test_repo_hashes_mismatch(tmp_path): for repo_path in [repo1_path, repo2_path]: repo_path.mkdir() - subprocess.run(['git', 'init'], cwd=repo_path, capture_output=True) - subprocess.run(['git', 'config', 'user.name', 'Test'], cwd=repo_path, capture_output=True) + subprocess.run(['git', 'init'], cwd=repo_path, capture_output=True, check=True) + subprocess.run( + ['git', 'config', 'user.name', 'Test'], + cwd=repo_path, + capture_output=True, + check=True, + ) subprocess.run( ['git', 'config', 'user.email', 'test@test.com'], cwd=repo_path, capture_output=True, + check=True, ) (repo_path / 'file.txt').write_text('content') - subprocess.run(['git', 'add', 'file.txt'], cwd=repo_path, capture_output=True) - subprocess.run(['git', 'commit', '-m', 'Initial'], cwd=repo_path, capture_output=True) + subprocess.run(['git', 'add', 'file.txt'], cwd=repo_path, capture_output=True, check=True) + subprocess.run( + ['git', 'commit', '-m', 'Initial'], + cwd=repo_path, + capture_output=True, + check=True, + ) (repo2_path / 'file2.txt').write_text('content2') - subprocess.run(['git', 'add', 'file2.txt'], cwd=repo2_path, capture_output=True) - subprocess.run(['git', 'commit', '-m', 'Second'], cwd=repo2_path, capture_output=True) + subprocess.run(['git', 'add', 'file2.txt'], cwd=repo2_path, capture_output=True, check=True) + subprocess.run( + ['git', 'commit', '-m', 'Second'], + cwd=repo2_path, + capture_output=True, + check=True, + ) with pytest.raises(ValueError, match='does not match'): compare_repo_commit_hashes( diff --git a/tox.ini b/tox.ini index 7430a60f..fa8cde72 100644 --- a/tox.ini +++ b/tox.ini @@ -49,7 +49,7 @@ commands = description = Check our style guide labels = check deps = - ruff + ruff == 0.16.0 skip_install = true commands = ruff check --diff @@ -59,7 +59,7 @@ commands = description = Auto-apply style guide to the extent possible labels = pre-release deps = - ruff + ruff == 0.16.0 skip_install = true commands = ruff check --fix @@ -99,4 +99,4 @@ deps = twine skip_install = true commands = - python -m twine upload dist/* \ No newline at end of file + python -m twine upload dist/*