Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Remove label parameter where it is not needed #602

Merged
merged 21 commits into from
Oct 26, 2020

Conversation

SabineHaas
Copy link
Collaborator

@SabineHaas SabineHaas commented Oct 8, 2020

Fix #375

Changes proposed in this pull request:

  • Delete unnecessary parameter label from input csv files; label is now set by filenames (for project_data, economic_data, simulation_settings) and column names (for energyConsumption, energyConversion, energyProduction, energyProviders), special for storage: filename + column name
  • Note in documentation that the column headers need to be unique amongst all files

The following steps were realized, as well (if applies):

  • Use in-line comments to explain your code
  • Write docstrings to your code (example docstring)
  • For new functionalities: Explain in readthedocs
  • Write test(s) for your new patch of code (pytests, assertion debug messages)
  • Update the CHANGELOG.md
  • Apply black (black . --exclude docs/)
  • Check if benchmark tests pass locally (EXECUTE_TESTS_ON=master pytest)

Please mark above checkboxes as following:

  • Open
  • Done

❌ Check not applicable to this PR

For more information on how to contribute check the CONTRIBUTING.md.

@SabineHaas SabineHaas self-assigned this Oct 8, 2020
@SabineHaas
Copy link
Collaborator Author

@smartie2076, I left fixcost.csv as it is, as it is not used. If it's planned to be used in the future we could use the column headers as labels.

@SabineHaas SabineHaas marked this pull request as ready for review October 21, 2020 12:53
@smartie2076
Copy link
Collaborator

@smartie2076, I left fixcost.csv as it is, as it is not used. If it's planned to be used in the future we could use the column headers as labels.

Ok

@Bachibouzouk
Copy link
Collaborator

* Delete unnecessary parameter label from input csv files; label is now set by filenames (`project_data`, `economic_data`, `simulation_settings`) and column names (`energyConsumption`, `energyConversion`, `energyProduction`, `energyProviders`), special for storage: `filename` + `column name`

aren't energyConsumption etc.. also filenames?

@SabineHaas
Copy link
Collaborator Author

* Delete unnecessary parameter label from input csv files; label is now set by filenames (`project_data`, `economic_data`, `simulation_settings`) and column names (`energyConsumption`, `energyConversion`, `energyProduction`, `energyProviders`), special for storage: `filename` + `column name`

aren't energyConsumption etc.. also filenames?

I added the word "for" to make it clearer:
Delete unnecessary parameter label from input csv files; label is now set by filenames (for project_data, economic_data, simulation_settings) and column names (for energyConsumption, energyConversion, energyProduction, energyProviders), special for storage: filename + column

I would add this to the changelog if you agree.

Copy link
Collaborator

@Bachibouzouk Bachibouzouk left a comment

Choose a reason for hiding this comment

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

Did you run the test with EXECUTE_TESTS_ON=master? if yes, then I approve the PR

@SabineHaas
Copy link
Collaborator Author

Did you run the test with EXECUTE_TESTS_ON=master? if yes, then I approve the PR

Yes, I did. Thanks for reviewing!

@Bachibouzouk
Copy link
Collaborator

If it is your own branch, you can do rebase instead of merging dev into the branch :)

@SabineHaas
Copy link
Collaborator Author

If it is your own branch, you can do rebase instead of merging dev into the branch :)

with rebase I got a merge conflict, while with merging I didn't..

@SabineHaas SabineHaas merged commit 2a24a72 into dev Oct 26, 2020
@SabineHaas SabineHaas deleted the feature/remove_label_parameter branch October 26, 2020 16:15
@smartie2076 smartie2076 mentioned this pull request Nov 10, 2020
smartie2076 added a commit that referenced this pull request Nov 12, 2020
Parameter label from input csv files; label is now set by filenames (for `project_data`, `economic_data`, `simulation_settings`) and column headers (for `energyConsumption`, `energyConversion`, `energyProduction`, `energyProviders`), special for storage: `filename` + `column header` (#602)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Label might be an unneeded parameter in the csvs
3 participants