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

Guardrail for MCNP_Problem.write_to_file() #442

Closed
tjlaboss opened this issue Jul 10, 2024 · 1 comment · Fixed by #443
Closed

Guardrail for MCNP_Problem.write_to_file() #442

tjlaboss opened this issue Jul 10, 2024 · 1 comment · Fixed by #443
Assignees
Labels
feature request An issue that improves the user interface.

Comments

@tjlaboss
Copy link
Collaborator

New function signature:

def write_to_file(self, new_problem, overwrite=False):
    """
    :param overwrite: Whether to overwrite the file at 'new_problem' if it exists
    :type overwrite: bool
    """
    . . .

Describe the solution you'd like
Check for the existence of the file new_problem, and do not overwrite it unless overwrite=True.

One solution: simply raise an error.

if overwrite or not os.path.exists(new_problem):
    # write the file as normal
    return
elif os.path.isfile(new_problem):
    raise FileExistsError(new_problem)
elif os.path.isdir(new_problem):
    raise IsDirectoryError(new_problem):
. . .

Describe alternatives you've considered
Or, if the file exists and overwrite == False, back up the original file with some sort of identifier, and then write to the requested filename.

UUID:

def get_backup_fname(new_problem):
    base_name, extension = os.path.splitext(new_problem)
    unique_id = uuid.uuid4()
    new_fname = f"{base_name}_backup_{unique_id}{extension}"
    return new_fname

Timestamp:

def get_next_backup_fname(new_problem):
    base_name, extension = os.path.splitext(new_problem)
    timestamp = int(time.time())   # OR use strftime('%Y%m%d%H%M%S') for human-readable
    new_fname = f"{base_name}_backup_{timestamp}{extension}"

    # If user going faster than one file per second
    index = 1
    while os.path.exists(new_file):
        new_file = f"{base_name}_backup_{timestamp}_{index}{extension}"
        index += 1

    return new_fname

Additional context
Related to pyOpenSci review here: pyOpenSci/software-submission#205 (comment)

@tjlaboss tjlaboss added the feature request An issue that improves the user interface. label Jul 10, 2024
@MicahGale
Copy link
Collaborator

Ok I see value in this. I think I'd prefer to just error out rather than creating new files that user wasn't expecting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request An issue that improves the user interface.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants