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

chore: create a design folder for capacity planning, design transmission, scenario info #124

Merged
merged 4 commits into from
Apr 8, 2020

Conversation

dmuldrew
Copy link
Collaborator

@dmuldrew dmuldrew commented Apr 2, 2020

@ben, @danielolsen, @BainanXia, this PR just includes folder restructuring we discussed:

create powersimdata/design/ folder and move the generation capacity framework along with theScenarioInfo object therein. The design_transmission module will be moved there too --> 1 PR

rouille
rouille previously approved these changes Apr 2, 2020
Copy link
Collaborator

@rouille rouille left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for doing that!

Try to use present tense in your commit message in the future. This is the convention we came up with:
https://github.com/intvenlab/REISE/wiki/Software-Development-Guidelines

@rouille rouille dismissed their stale review April 2, 2020 22:35

New folder design required

@rouille
Copy link
Collaborator

rouille commented Apr 3, 2020

It seems like you did not move powersimdata/input/tests/test_design_transmission.py

@rouille
Copy link
Collaborator

rouille commented Apr 3, 2020

Also, you will need to update the following:

powersimdata/input/change_table.py:4:from powersimdata.input.design_transmission import (
powersimdata/input/change_table.py:299:        Optional kwargs as documented in powersimdata.input.design_transmission
powersimdata/input/change_table.py:307:        Optional kwargs as documented in powersimdata.input.design_transmission

@BainanXia
Copy link
Collaborator

test_scenario_info.py needs to be moved accordingly as well...

@dmuldrew
Copy link
Collaborator Author

dmuldrew commented Apr 3, 2020

I've taken care of moving the test files mentioned above and updating change table import paths. Looks like there's still an issue with the ScenarioInfo tests failing, @BainanXia can you take a look at that...

@dmuldrew dmuldrew mentioned this pull request Apr 3, 2020
@BainanXia
Copy link
Collaborator

@dmuldrew It's just about the import paths update. Fixed for both test_scenario_info.py and test_design_transmission.

@rouille
Copy link
Collaborator

rouille commented Apr 3, 2020

All tests pass.

Can you do an interactive rebase to keep only essential commit messages (fixup command) using git rebase -i HEAD~7. Please, start the message with a verb in the present tense (see https://github.com/intvenlab/REISE/wiki/Software-Development-Guidelines). Thanks.

@kasparm
Copy link
Contributor

kasparm commented Apr 3, 2020

README.md adjustments needed.

@dmuldrew dmuldrew force-pushed the folder_reorganization branch 2 times, most recently from 7762ffc to ceac4da Compare April 7, 2020 05:05
@dmuldrew dmuldrew force-pushed the folder_reorganization branch from d0c0ae7 to 7242120 Compare April 7, 2020 20:32
@danielolsen
Copy link
Contributor

danielolsen commented Apr 7, 2020

Please rename powersimdata/design/design_transmission.py to powersimdata/design/transmission.py (and change associated imports).

Please rename powersimdata/design/demo/data/Eastern Scenario Take 2.csv to something more meaningful.

@rouille
Copy link
Collaborator

rouille commented Apr 7, 2020

The __all__ variable needs to be updated with the module names in design.
The clean_capacity_scaling module has lot of docstrings where type of argumentsare not specified, have missing arguments or do not follow our convention.

@BainanXia
Copy link
Collaborator

As we discussed before, there should be a data folder in the tests folder to handle all test related data files as well. Also, @danielolsen 's requests on renaming files are not addressed. Based on the decision @rouille mentioned in the meeting, the docstrings of clean_capacity_scaling is going to be fixed in another PR after this.

@dmuldrew
Copy link
Collaborator Author

dmuldrew commented Apr 8, 2020

I put a data folder in demo because it was needed for the demo notebook:
https://github.com/intvenlab/PowerSimData/tree/folder_reorganization/powersimdata/design/demo
None of my tests use external data files, so I'm not sure it makes sense to create an data empty folder.

I'll take care of the file rename for Daniel...

@BainanXia
Copy link
Collaborator

None of my tests use external data files

What does test_case_input_output.csv do? If we don't need it, then we should remove it from the package.

@dmuldrew
Copy link
Collaborator Author

dmuldrew commented Apr 8, 2020

OK, I see now...I created that earlier and never used it. I'll remove it.

@rouille
Copy link
Collaborator

rouille commented Apr 8, 2020

It looks like you did not change the import statements, __init__ and test file after renaming design_transmission to transmission:

  • test_design_transmission.py --> test_transmission.py
  • powersimdata/design/tests/test_design_transmission.py:from powersimdata.design.design_transmission import (
  • powersimdata/input/init.py:all = ['abstract_grid', 'change_table', 'csv_reader', 'design_transmission', --> needs to be removed
  • powersimdata/input/change_table.py:from powersimdata.design.design_transmission import (
  • powersimdata/input/change_table.py: Optional kwargs as documented in powersimdata.design.design_transmission
  • powersimdata/input/change_table.py: Optional kwargs as documented in powersimdata.design.design_transmission

This is why the tests are failing:

(v1) [~/REM/PowerSimData] (folder_reorganization) brdo$ pytest . --verbose
================================================================ test session starts =================================================================
platform darwin -- Python 3.6.5, pytest-4.5.0, py-1.8.0, pluggy-0.13.0 -- /Users/brdo/PyEnv/REM/v1/bin/python
cachedir: .pytest_cache
rootdir: /Users/brdo/REM/PowerSimData
collected 55 items / 2 errors / 53 selected                                                                                                          

======================================================================= ERRORS =======================================================================
_______________________________________ ERROR collecting powersimdata/design/tests/test_design_transmission.py _______________________________________
ImportError while importing test module '/Users/brdo/REM/PowerSimData/powersimdata/design/tests/test_design_transmission.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
powersimdata/design/tests/test_design_transmission.py:7: in <module>
    from postreise.tests.mock_scenario import MockScenario
../PostREISE/postreise/tests/mock_scenario.py:1: in <module>
    from powersimdata.scenario.scenario import Scenario
powersimdata/scenario/scenario.py:6: in <module>
    from powersimdata.scenario.create import Create
powersimdata/scenario/create.py:5: in <module>
    from powersimdata.input.change_table import ChangeTable
powersimdata/input/change_table.py:4: in <module>
    from powersimdata.design.design_transmission import (
E   ModuleNotFoundError: No module named 'powersimdata.design.design_transmission'
__________________________________________ ERROR collecting powersimdata/design/tests/test_scenario_info.py __________________________________________
ImportError while importing test module '/Users/brdo/REM/PowerSimData/powersimdata/design/tests/test_scenario_info.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
powersimdata/design/tests/test_scenario_info.py:5: in <module>
    from postreise.tests.mock_scenario import MockScenario
../PostREISE/postreise/tests/mock_scenario.py:1: in <module>
    from powersimdata.scenario.scenario import Scenario
powersimdata/scenario/scenario.py:6: in <module>
    from powersimdata.scenario.create import Create
powersimdata/scenario/create.py:5: in <module>
    from powersimdata.input.change_table import ChangeTable
powersimdata/input/change_table.py:4: in <module>
    from powersimdata.design.design_transmission import (
E   ModuleNotFoundError: No module named 'powersimdata.design.design_transmission'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 2 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
============================================================== 2 error in 2.62 seconds ===============================================================

Copy link
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

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

This is nice. All tests passed and the folders are reorganized based on our proposal. Thanks!

@rouille rouille force-pushed the folder_reorganization branch from e937a15 to 7673b05 Compare April 8, 2020 23:40
@rouille rouille merged commit 0470133 into develop Apr 8, 2020
@rouille rouille deleted the folder_reorganization branch April 8, 2020 23:55
@rouille
Copy link
Collaborator

rouille commented Apr 9, 2020

@dmuldrew had to pass the batton since he had to finish writing out the code for the grid scaling framework. I did the following before merging this PR:

  • update import statements throughout PowerSimData after the powersimdata.design.design_transmission module has been renamed as powersimdata.design.transmission.
  • rename Eastern Scenario Take 2.csv to eastern_2030_clean_energy_targets.csv and EasternCleanCapacityScaling.ipynb to eastern_clean_capacity_scaling_demo.ipynb. Also update import statements in notebook and input csv file.
  • remove unnecessary __init__ modules in demo and data folders.
  • update __all__ in powersimdata.input.__init__ after moving module.
  • update docstring in powersimdata.design.clean_capacity_scaling module

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.

5 participants