Skip to content

Refactor csv export#401

Merged
chrwm merged 29 commits intodevelopfrom
refactor-400-csv-export
Jan 19, 2023
Merged

Refactor csv export#401
chrwm merged 29 commits intodevelopfrom
refactor-400-csv-export

Conversation

@chrwm
Copy link
Copy Markdown
Member

@chrwm chrwm commented Dec 15, 2022

Closes #400

chrwm added 8 commits December 15, 2022 19:41
The feature has been depreciated from Markstammdatenregister
* rename self.--- objects
* move constants to constants.py
* was not tested
* rename self.--- objects
* move constants to constants.py
@chrwm chrwm added the ♻️ refactor Refactoring of existing code for better maintainability label Dec 15, 2022
@chrwm chrwm self-assigned this Dec 15, 2022
@chrwm chrwm changed the title Refactor 400 csv export Refactor csv export Dec 15, 2022
chrwm added 16 commits December 16, 2022 17:55
Can not be improved as e.g. `orm.BasicUnit.EinheitMastrNummer == orm_tables["unit_data"].EinheitMastrNummer` EinheitenMastrNummer has to be called with .
* all csv tables are exported via one export function
* all db queries are created in one function `create_db_query`. The structure of the function should enable easy tables joins in the future for csv export within on session.
* additional tables can be now also exported with a limit
* in constants.py ORM_MAP was extended. ORM_MAP maps the parameters that the user inserts to the tables classes in orm.py. This means additional tables for export can be easily added here, if desired.
* metadata saving was moved to its own function
* generalize csv-export logging info
…xport

# Conflicts:
#	open_mastr/soap_api/mirror.py
@chrwm chrwm marked this pull request as ready for review January 5, 2023 14:25
@chrwm
Copy link
Copy Markdown
Member Author

chrwm commented Jan 5, 2023

A short summary of the most important changes:
3 main functions exist:

  1. csv-export is triggered via MaStR.to_csv(): defines and collects tables to export
  2. open_mastr.utils.helpers.create_db_query: creates db-queries for TECHNOLOGIES and ADDITIONAL_TABLES
  3. open_mastr.utils.helpers.db_query_to_csv: exports db-query to csv for TECHNOLOGIES and ADDITIONAL_TABLES
  • functions relevant to the csv_export are now located in helpers.py

Pytest
Only open_mastr.utils.helpers.db_query_to_csv is tested for:

The test checks for 2 random tech and 2 random additional tables:
1. If csv's have been created and encoded in 'utf-8' and are not empty
2. For techs, if limited (limit=60) EinheitMastrNummer in basic_units are included in CSV file
3. For additional_tables, if csv is not empty

It is due to the Markstammdatenregister has unstable columns (columns are renamed, new columns are added, columns are deleted). This makes testing with static columns impractical in my opinion.
@FlorianK13 @deniztepe if you have ideas for better testing, please make suggestions.

@chrwm chrwm requested review from FlorianK13 and deniztepe January 5, 2023 16:38
@chrwm
Copy link
Copy Markdown
Member Author

chrwm commented Jan 5, 2023

Volkswagen was not necessary ;)

Copy link
Copy Markdown
Contributor

@deniztepe deniztepe left a comment

Choose a reason for hiding this comment

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

Overall it works well. I guess, the commented three points are small issues.

Comment thread open_mastr/utils/config.py
Comment thread open_mastr/utils/helpers.py
Comment thread tests/test_helpers.py Outdated
chrwm added 2 commits January 12, 2023 16:03
Currently metadata is only and can only be created for technologies. Will be enabled for additional_tables in #386
@deniztepe deniztepe mentioned this pull request Jan 13, 2023
2 tasks
* _db_path was getting longer if the folder has more files
* black applied
@chrwm
Copy link
Copy Markdown
Member Author

chrwm commented Jan 16, 2023

@deniztepe you can approve, if I met your suggestions.

@deniztepe deniztepe self-requested a review January 17, 2023 07:07
@chrwm chrwm merged commit 95e32a0 into develop Jan 19, 2023
@chrwm chrwm deleted the refactor-400-csv-export branch January 19, 2023 21:54
@nesnoj
Copy link
Copy Markdown
Collaborator

nesnoj commented Jan 20, 2023

Hey @chwrwm, I'd like to reference the package prior to the CSV refactoring. However, the latest version 0.12.1 does not include all recent fixes.
It would be very nice if you could prepare an in-between-release (at 1ba8c7c) without the CSV stuff.
Is that possible for you? If not, I could take take of this..

@nesnoj
Copy link
Copy Markdown
Collaborator

nesnoj commented Jan 20, 2023

Hey @chwrwm, I'd like to reference the package prior to the CSV refactoring. However, the latest version 0.12.1 does not include all recent fixes. It would be very nice if you could prepare an in-between-release without the CSV stuff. Is that possible for you? If not, I could take take of this..

Hmm, maybe it is not necessary. Did the CSV format change with the refactoring?

@FlorianK13 FlorianK13 removed their request for review January 20, 2023 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

♻️ refactor Refactoring of existing code for better maintainability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants