Skip to content
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

Created dso rename command #41

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Created dso rename command #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99c5ec8
initialised dso rename command
DSchreyer Oct 24, 2024
bf036df
pre-commit autofixes
DSchreyer Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/dso/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from .get_config import cli as get_config_cli
from .init import cli as init_cli
from .lint import cli as lint_cli
from .rename import cli as rename_cli
from .repro import cli as repro_cli
from .watermark import cli as watermark_cli

Expand Down Expand Up @@ -60,3 +61,4 @@ def cli(quiet: int, verbose: bool):
cli.add_command(lint_cli)
cli.add_command(get_config_cli)
cli.add_command(watermark_cli)
cli.add_command(rename_cli)
58 changes: 58 additions & 0 deletions src/dso/rename.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
"""Renames an existing stage or folder and update files accordingly"""

import sys
from os import getcwd
from pathlib import Path

import rich_click as click
from rich.prompt import Prompt

from dso._logging import log
from dso._util import get_project_root

check_files = ["params.in.yaml", "dvc.yaml"]
check_directories = [
"src",
]


def update_references(project_root: Path, old_name: str, new_name: str):
"""Update all references to the old name in the specified project files."""
for file_path in project_root.rglob("*"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to update the entire project? it might be more cautious to just change things within the affected stage, at the risk that we miss something.

Consider e.g. this directory structure and renaming RNAseq/01_preprocessing to RNAseq/02_preprocessing

.
├── RNAseq
│   └── 01_preprocessing
└── TSO500
    └── 01_preprocessing

or renaming 01_taskA to 01_taskB.

.
├── 01_taskA
└── 01_taskA_followup

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For case (2) it could help to regex match on a "whole word", but this doesn't save you in case (1).

if file_path.is_file() and (
any(part in file_path.parts for part in check_directories) or file_path.name in check_files
):
try:
content = file_path.read_text()
updated_content = content.replace(old_name, new_name)
file_path.write_text(updated_content)
except Exception as e:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's bad practice in general to catch Exception because it also contains exceptions you don't want to catch (e.g. KeyboardInterrupt. A good candidate here could be OSError.

log.error(f"[red]Failed to update {file_path}: {e}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to continue in such a case or stop the program? It seems weird that there are first failures and then the main function prints out Renamed xxx successfully.

An option would be to count errors and return the error count / list of errors and react in the main function accordingly.



@click.argument("old_name", required=False)
@click.argument("new_name", required=False)
@click.command(
"rename",
)
def cli(old_name: str | None = None, new_name: str | None = None):
"""Rename an existing stage or folder and update files accordingly."""
if old_name is None:
old_name = Prompt.ask("[bold]Please enter the directory name of the stage or folder that shall be renamed")
if new_name is None:
new_name = Prompt.ask("[bold]Please enter the new stage or folder name")
project_root = get_project_root(Path(getcwd()))
old_path = project_root / old_name
new_path = project_root / new_name
Comment on lines +39 to +46
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all relative to the project root now. I would find it more intuitive if it was relative to the CWD, because or basically any shell tool works like that.


if not old_path.exists():
log.error(f"[red]{old_name} does not exist!")
sys.exit(1)

if new_path.exists():
log.error(f"[red]{new_name} already exists!")
sys.exit(1)

old_path.rename(new_path)
update_references(project_root, old_name, new_name)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function should additionally take care of renaming files within a stage, e.g. src/01_stage_name.qmd into src/new_stage_name.qmd. Basically any file within a stage directory that's called {stage_name}.*.

log.info(f"[green]Renamed from {old_name} to {new_name} successfully.")