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.

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

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

wants to merge 2 commits into from

Conversation

DSchreyer
Copy link
Collaborator

@DSchreyer DSchreyer commented Oct 24, 2024

This is the first draft of a dso rename command. It takes a stage / folder name into account and renames it. It also renames files accordingly.

Still missing

  • dso rename in dso function overview - when typing dso rename does not pop up -> However, dso rename works as expected
  • needs to be clarified which folders or scripts shall be renamed accordingly.

Copy link
Contributor

PR Preview Action v1.4.8
🚀 Deployed preview to https://Boehringer-Ingelheim.github.io/dso/pr-preview/pr-41/
on branch gh-pages at 2024-10-24 10:05 UTC

@DSchreyer DSchreyer requested a review from grst October 24, 2024 10:57
@tschwarzl
Copy link
Contributor

I would like to discuss if it would be of interest to automatically stash those changes for all git tracked files as well.

Copy link
Collaborator

@grst grst left a comment

Choose a reason for hiding this comment

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

Thanks for taking the initiative. I think there's a lot of things that can wrong during renaming, and we should try to make this a bit more robust. I'd err on the side of rather reanaming too little than too much.

Also, we'll need tests for this function.

Comment on lines +39 to +46
"""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
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.

updated_content = content.replace(old_name, new_name)
file_path.write_text(updated_content)
except Exception as e:
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.

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.


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).

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}.*.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants