polca / premise

Coupling Integrated Assessment Models output with Life Cycle Assessment.
BSD 3-Clause "New" or "Revised" License
100 stars 45 forks source link

added_include_time_option #161

Closed Haitham-ghaida closed 2 months ago

Haitham-ghaida commented 3 months ago

Hi,

Calling functions like generate_scenario_report or generate_change_report multiple times within the same day results in the subsequent executions overwriting the previously generated files. This behavior, as discussed in issue #160 , requires manual backup actions by users who wish to preserve each generated report.

Since file generation is a common task across various functions within the library, I propose introducing a generate_filename utility function. This function resides within the utils module and offers a standardized approach to filename creation. It accepts a base_name and a boolean flag include_time. When include_time is True, the function appends a timestamp to the filename; otherwise, it only includes the date.

This enhancement has been integrated into the generate_scenario_report and generate_change_report functions through the addition of an include_time parameter, defaulting to False. This maintains backward compatibility with existing workflows while offering users the option to include timestamps in filenames to prevent overwrites. The feature extends to other file-generating functions, such as write_db_to_brightway and write_db_to_matrices, ensuring a consistent and flexible approach to filename generation across the library.

I've tested these changes to ensure they function as expected.

This update should only affect standard users' workflows if the new include_time option is explicitly utilized.

Looking forward to your feedback :)

Haitham-ghaida commented 2 months ago

another option can be to add this function or something like it


def increment_filename(filepath: Path) -> Path:
    """
    Increments a filename to avoid overwriting existing files.

    If the file "report.xlsx" exists, this will try "report_1.xlsx", "report_2.xlsx", etc.
    """
    if not filepath.exists():
        return filepath

    directory = filepath.parent
    name = filepath.stem
    extension = filepath.suffix

    counter = 1
    new_filepath = filepath
    while new_filepath.exists():
        new_filepath = directory / f"{name}_{counter}{extension}"
        counter += 1

    return new_filepath

and then in the generate_scenario_report call it full_path = increment_filename(filepath / name) but this way it doesn't give the user the choice to opt out if they don't care about preserving these reports

romainsacchi commented 2 months ago

Sorry @Haitham-ghaida, just saw this a bit too late. Commit https://github.com/polca/premise/commit/e0b76d50c8cad2c6f69520b5a87ca37901695e4d now fixes this I believe. It adds the hour of the day, which ensures that scenario difference files cannot be overwritten in principle.