PowerGridModel / power-grid-model-io

Conversion tool for various grid data formats to power-grid-model
Mozilla Public License 2.0
12 stars 8 forks source link

[FEATURE] *convert_guid_vision_excel Converter access* #245

Closed jaapschoutenalliander closed 4 months ago

jaapschoutenalliander commented 4 months ago

Provide possible access to UUID2IntCvtr object

I would like to be able to access the mapping between generated pgm IDs and GUIDs as created bij the UUID2IntCvtr. To this I would like to be able to optionally provide the UUID2IntCvtr object to the function. This can be used to access cvrt._uuids_int Proposal:

def convert_guid_vision_excel(
    excel_file: Union[Path, str],
    number: str = VISION_EXCEL_LAN_DICT[LANGUAGE_EN][DICT_KEY_NUMBER],
    terms_changed: Optional[dict] = None,
    cvtr: Optional[UUID2IntCvtr] = None,
) -> str:
    """Main entry function. Convert the GUID based Vision excel files to a number based format

    Args:
        excel_file (Path | str): Vision excel file name
        number (str): "Number" or "Nummer" depending on the language. Defaults to "Number".
        terms_changed (dict): the dictionary containing the terms to be changed. Defaults to {}.

    Returns:
        str: the new excel file name
    """
    if terms_changed is None:
        terms_changed = {}
    xls = load_excel_file(excel_file)
    cvtr = cvtr or UUID2IntCvtr()
    dir_name, file_name = os.path.split(excel_file)
    new_excel_name = os.path.join(dir_name, f"new_{file_name}")

    for i, sheet_name in enumerate(xls.sheet_names):
        print(i)
        df = xls.parse(sheet_name)
        update_column_names(df, terms_changed)
        guid_columns = get_guid_columns(df)

        for guid_column in guid_columns:
            add_guid_values_to_cvtr(df, guid_column, cvtr)
            insert_or_update_number_column(df, guid_column, sheet_name, cvtr, number)

        save_df_to_excel(df, new_excel_name, sheet_name, i)

    return new_excel_name
mgovers commented 4 months ago

I think this should be OK to add. @Jerry-Jinfeng-Guo do you see any issue with this?

If @Jerry-Jinfeng-Guo is alright with it, then you @jaapschoutenalliander can go ahead and create a PR for this.

Jerry-Jinfeng-Guo commented 4 months ago

Hi @jaapschoutenalliander Thanks for the request. If I understand it correctly, the ultimate goal is that you would like to have the original/corresponding UUID information available. We have a pending issue on including the UUID natively to the extra_info field of the converted result. If the description above fits your need already, I think we'll just have to prioritize a bit to deliver earlier than planned. If you are indeed in a hurry and what I mentioned above won't be enough, let's discuss this feature request further.

jaapschoutenalliander commented 4 months ago

@Jerry-Jinfeng-Guo Perfect I think that would suffice, when would you be able to add this?

petersalemink95 commented 4 months ago

@jaapschoutenalliander Thanks for the request. We can implement the solution suggested by @Jerry-Jinfeng-Guo next sprint.

Jerry-Jinfeng-Guo commented 4 months ago

See https://github.com/PowerGridModel/power-grid-model-io/pull/249

jaapschoutenalliander commented 4 months ago

Thank you, I will close the issue