63 clean code and refactor into class excel to entities

[carlosmada22]

This pull request includes changes to the .vscode/settings.json file to add a new debug configuration, and modifications to the bam_masterdata module to refactor the way Excel data is extracted. The most important changes are summarized below:

Debug Configuration Improvements:

• .vscode/settings.json: Added a new debug configuration for exporting to RDF and set justMyCode to false to include library code in debugging. [1] [2]

Code Refactoring:

• bam_masterdata/cli/fill_masterdata.py: Refactored to use MasterdataExcelExtractor for extracting entities from Excel, replacing the previous excel_to_entities function. [1] [2]
• bam_masterdata/excel/__init__.py: Added import for MasterdataExcelExtractor to the module's __init__.py file.

[coveralls]

Pull Request Test Coverage Report for Build 13112718684

Details

• 22 of 188 (11.7%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.8%) to 96.257%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
bam_masterdata/cli/fill_masterdata.py	1	2	50.0%
bam_masterdata/excel/excel_to_entities.py	21	186	11.29%

Totals
Change from base Build 13052092592:	0.8%
Covered Lines:	16407
Relevant Lines:	17045

---

💛 - Coveralls

[JosePizarro3]

Looks perfect, congrats! 🙂

Just a couple of comments for you to discuss with me if you agree or not.

Maybe it is a good idea to add testing for this class, but this I leave for you to decide if you want to do it in this PR or open a new issue and tackle it later.

[JosePizarro3]

You can use the Enum defined in bam_masterdata.metadata.definitions import DataType. If you want to check if some value is allowed by the data types, you just need to do:

value in [dt.value for dt in DataType]

[carlosmada22]

Changed thank you!

[JosePizarro3]

Can you add a todo comment before this variable?

# TODO move these validation rules to a separate json

This is because I think we could have these mappings in a separate JSON to make it more readable. But you can leave it for another pr :)

[JosePizarro3]

Ok, I checked the code and it seems Worksheet is only used for typing. You can:

import re
from typing import TYPE_CHECKING, Any, Optional, Union

if TYPE_CHECKING:
    from openpyxl.worksheet.worksheet import Worksheet

Then, you need to change in all the times a Worksheet was typed in a function by "Worksheet", i.e., add the double-quotes, e.g.:

    def get_last_non_empty_row(
        self, sheet: "Worksheet", start_index: int
    ) -> Optional[int]:

The thing is that, if a class is only used for typing checks, then it does not need to be imported (and hence added to memory) every time the module is executed. That's why we have this TYPE_CHECKING and we just need to add an if condition + double quotes when typing the specific class.

[carlosmada22]

Maybe it is a good idea to add testing for this class, but this I leave for you to decide if you want to do it in this PR or open a new issue and tackle it later.

Perfect, I will open a new issue for the testing after this pull request.