Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Feature 308 transform technology parameter #337

Merged
merged 13 commits into from
Aug 29, 2022
Merged

Feature 308 transform technology parameter #337

merged 13 commits into from
Aug 29, 2022

Conversation

deniztepe
Copy link
Contributor

This pull request is composed of many file changes. Although all tests are passed, please try to review carefully and comment if something is unclear or not optimal. Thanks

Summary

  • Data parameter validation differs between methods
  • Renaming: Technology -> data
  • Validation includes now also empty lists as inputs
  • Data parameter is parsed on Mastr level and lower functions are expecting only list.
  • Test to_csv is fastened

@deniztepe
Validate parameter data and differentiate between api and bulk data #308
ef936f2
@deniztepe
Rename technology to data #308
d2f3ac0 
The parameter 'technology' is replaced with 'data' wherever it means more than only technologies.
It requires a thorough scan of scripts for a consistent refactoring.
The documentation must be adapted as well.
@deniztepe
Shorten the test to_csv
9e9778c 
It takes too long if you want to test with a filled open-mastr database
@deniztepe
Fasten search using sets instead of lists
13e768a 
*Increase chunksize
*https://stackoverflow.com/questions/5993621/fastest-way-to-search-a-list-in-python
@deniztepe
Validate that list parameters are not empty lists #308
f53db88 
[None], [] are not valid anymore
@deniztepe
Transform and harmonize data input #308
9fe4e88 
*api_data_types and api_location_types default now to all possible selections.
* transform function returns lists required from lower functions -> lower functions must handle only lists -> no parsing etc.
@deniztepe
Add test #308
2d188bb
@deniztepe
Remove redundant parsing of data parameter #308
99f4fda 
* Black applied
@deniztepe deniztepe requested review from chrwm and FlorianK13 August 22, 2022 15:14
@deniztepe deniztepe self-assigned this Aug 22, 2022
@deniztepe
Adapt documentation for data parameter #308
110ed24 
* Technology is renamed to data where necessary
* In docstring, a table for possible data values is added
@deniztepe deniztepe marked this pull request as ready for review August 24, 2022 08:36
@deniztepe deniztepe linked an issue Aug 24, 2022 that may be closed by this pull request
Add functions for parameter validity check and parameter transformation #308
Closed
@chrwm
Copy link
Member

chrwm commented Aug 24, 2022

This pull request is composed of many file changes. Although all tests are passed, please try to review carefully and comment if something is unclear or not optimal. Thanks

In this case, I'll take the time on Friday. Thank you

FlorianK13
FlorianK13 approved these changes Aug 26, 2022
View reviewed changes
Copy link
Member

@FlorianK13 FlorianK13 left a comment

Choose a reason for hiding this comment

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

Seems good. We anyway have to increase manual testing before each release.

chrwm
chrwm requested changes Aug 26, 2022
View reviewed changes
Copy link
Member

@chrwm chrwm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Overall it looks fine. I found one bug in the soap_API tqdm messages, which stems from the renaming of technology to data. I'll push a commit.

FlorianK13 and others added 2 commits August 29, 2022 13:09
@FlorianK13 @chrwm
Update tests/soap_api/test_mastr_mirror.py
d28e614 
Co-authored-by: chrwm <54852694+chrwm@users.noreply.github.com>
@FlorianK13 @chrwm
Update tests/soap_api/test_mastr_mirror.py
4b78b43 
Co-authored-by: chrwm <54852694+chrwm@users.noreply.github.com>
@FlorianK13 FlorianK13 merged commit 9717527 into develop Aug 29, 2022
@FlorianK13 FlorianK13 deleted the feature-308-transform-technology-parameter branch August 29, 2022 11:18
@FlorianK13 FlorianK13 mentioned this pull request Aug 29, 2022
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@FlorianK13 FlorianK13 FlorianK13 approved these changes

@chrwm chrwm chrwm approved these changes

Assignees

@deniztepe deniztepe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add functions for parameter validity check and parameter transformation
3 participants
@deniztepe @chrwm @FlorianK13