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

Multi invest bug #42

Merged
merged 16 commits into from
Sep 11, 2024
Merged

Multi invest bug #42

merged 16 commits into from
Sep 11, 2024

Conversation

samgdotson
Copy link
Member

@samgdotson samgdotson commented Sep 10, 2024

This PR fixes the multi-investment issue where PyPSA was using generators from future years in earlier ones. Closes #40.

Why should we use pull requests to collaborate?

  1. Pull requests serve as a built-in QA/QC step when developing research code and software.
  2. The discussions within issues and pull requests document the rationale for certain decisions for future use.
  3. Critically reviewing other people's code (and having your own code critically reviewed) is a great way to improve coding skills.
  4. Pull requests don't have to be reviewed by someone working on the same project. Thus, PRs allow greater cross-program collaboration and break down silos.
  5. Similarly, PRs allow interested third parties to meaningfully contribute to open-source projects. In the case of UCS, that may include enthusiastic members of the science network.
  6. PRs protect your data and repository by preventing force pushes. In many cases, at least one other person must review the code before it is "merged" with the main code base.

Where to focus attention

The following files should be prioritized for review:

  • All files in the scripts/ folder (these should be python source files, 8 in total).
  • All files in the utils/ folder (these should be python source files, 2 in total).
  • The Snakefile file
  • The config.yml file

The jupyter notebook files can be ignored since they are experimental and will be removed in a future pull request.

How to review a pull request

This post goes over the steps for reviewing a pull request.

Recommended checklist for reviewing a pull request

The guide below is modified from the Advanced Reactors and Fuel Cycles (ARFC) research group at the University of Illinois Urbana-Champaign

  • Read the PR description
  • Read through all the changes, considering the following questions.
    • Is there anything you can praise about this PR? Start with
      praise.
    • Are variable names brief but descriptive?
    • Are new/changed functions no longer than a paragraph?
    • Do all function parameters have default values where appropriate?
    • Is the code clear and clean? (see Robert C. Martin's
      Clean Code
      • If something is unclear, ask for clarification
      • Can you offer a suggestion to make the functionality clearer?
    • Is there enough documentation?
    • Does the programming style meet the requirements of the
      repository (PEP8 for python,
      google C++ style guide, etc.)
    • If a new feature has been added, or a bug fixed, has a test been
      added to confirm good behavior?
    • Does the test actually test the new/changed functionality?
    • Does the test successfully test edge cases?
    • Does the test successfully test corner cases?
    • If the repository has continuous integration: does the PR pass
      the tests?
    • If there is no continuous integration, check out the branch
      locally, build, and run the tests.
    • Do the tests pass on your machine?
    • Is the PR free of random cruft (built files, .swp files,
      etc.)?
    • Do all files in the PR belong in the repository?
    • If the PR deletes files, is this appropriate?
    • If the PR adds files or data, are these new files compatible
      with the repository license?
    • Make a review, leaving kind comments and suggesting changes
      where needed (to resolve the above).
    • Has the author resolved all of the comments and suggestions
      in your review?
  • When you approve of the PR, merge and close it.
  • Does this PR close an issue? If so, be sure to descriptively
    close this issue when the PR is merged
  • Thank the author for th

@samgdotson samgdotson added Type:Bug Something is wrong or broken. This issue or PR is related to a bug in code. Comp:Build This issue has to do with the build system of the code/doc (makefiles, cmake, install errors) Difficulty:2-Challenging This issue may be complex or require specialized skills. Priority:1-Critical This is the highest priority (i.e. it is blocking other work or facing a deadline). Status:5-In Review This issue has been handled, and the solution is being reviewed. (probably as a PR) labels Sep 10, 2024
@samgdotson samgdotson added this to the Final model run milestone Sep 10, 2024
@samgdotson samgdotson requested a review from lshaver September 10, 2024 19:10
@samgdotson samgdotson self-assigned this Sep 10, 2024
@pep8speaks
Copy link

pep8speaks commented Sep 10, 2024

Hello @samgdotson! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 17:5: W291 trailing whitespace
Line 19:1: E722 do not use bare 'except'
Line 21:1: W293 blank line contains whitespace
Line 23:80: E501 line too long (81 > 79 characters)
Line 24:1: E722 do not use bare 'except'
Line 399:1: W293 blank line contains whitespace
Line 401:1: W293 blank line contains whitespace
Line 403:80: E501 line too long (83 > 79 characters)
Line 404:1: W293 blank line contains whitespace
Line 411:1: W293 blank line contains whitespace
Line 413:1: W293 blank line contains whitespace
Line 415:29: E127 continuation line over-indented for visual indent
Line 416:29: E127 continuation line over-indented for visual indent
Line 417:29: E127 continuation line over-indented for visual indent
Line 418:29: E127 continuation line over-indented for visual indent
Line 419:29: E127 continuation line over-indented for visual indent
Line 420:29: E127 continuation line over-indented for visual indent
Line 421:29: E124 closing bracket does not match visual indentation
Line 424:1: W293 blank line contains whitespace
Line 430:1: W293 blank line contains whitespace
Line 432:1: W293 blank line contains whitespace
Line 435:1: W293 blank line contains whitespace
Line 441:29: E127 continuation line over-indented for visual indent
Line 442:29: E127 continuation line over-indented for visual indent
Line 443:29: E127 continuation line over-indented for visual indent
Line 444:29: E127 continuation line over-indented for visual indent
Line 445:29: E127 continuation line over-indented for visual indent
Line 446:29: E127 continuation line over-indented for visual indent
Line 447:29: E124 closing bracket does not match visual indentation
Line 450:1: W293 blank line contains whitespace
Line 453:1: E302 expected 2 blank lines, found 1
Line 456:66: W291 trailing whitespace
Line 460:1: W293 blank line contains whitespace
Line 462:1: W293 blank line contains whitespace
Line 464:1: W293 blank line contains whitespace
Line 472:25: E128 continuation line under-indented for visual indent
Line 473:25: E128 continuation line under-indented for visual indent
Line 474:25: E128 continuation line under-indented for visual indent
Line 475:25: E128 continuation line under-indented for visual indent
Line 476:25: E128 continuation line under-indented for visual indent
Line 477:25: E128 continuation line under-indented for visual indent
Line 480:1: W293 blank line contains whitespace
Line 482:1: W293 blank line contains whitespace
Line 542:5: E303 too many blank lines (2)
Line 544:1: W293 blank line contains whitespace
Line 567:80: E501 line too long (81 > 79 characters)

Line 11:1: E302 expected 2 blank lines, found 1
Line 21:45: E127 continuation line over-indented for visual indent
Line 22:45: E127 continuation line over-indented for visual indent
Line 24:1: W293 blank line contains whitespace
Line 25:80: E501 line too long (82 > 79 characters)
Line 27:1: W293 blank line contains whitespace
Line 28:42: E231 missing whitespace after ','

Line 18:38: W291 trailing whitespace
Line 19:1: W293 blank line contains whitespace
Line 24:1: W293 blank line contains whitespace
Line 184:1: W293 blank line contains whitespace
Line 186:33: E231 missing whitespace after ','
Line 186:80: E501 line too long (80 > 79 characters)
Line 188:80: E501 line too long (83 > 79 characters)
Line 189:80: E501 line too long (86 > 79 characters)
Line 190:80: E501 line too long (86 > 79 characters)
Line 191:80: E501 line too long (88 > 79 characters)
Line 194:1: W293 blank line contains whitespace
Line 195:71: E231 missing whitespace after ','
Line 195:80: E501 line too long (93 > 79 characters)
Line 208:43: E231 missing whitespace after ','
Line 251:5: E303 too many blank lines (2)

Comment last updated at 2024-09-11 16:26:45 UTC

scripts/add_electricity.py Outdated Show resolved Hide resolved
Co-authored-by: Sam Dotson <[email protected]>
Copy link
Contributor

@lshaver lshaver left a comment

Choose a reason for hiding this comment

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

With the suggested changes I approve this PR. Lots of good work here!

@lshaver lshaver merged commit 70831f8 into main Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Comp:Build This issue has to do with the build system of the code/doc (makefiles, cmake, install errors) Difficulty:2-Challenging This issue may be complex or require specialized skills. Priority:1-Critical This is the highest priority (i.e. it is blocking other work or facing a deadline). Status:5-In Review This issue has been handled, and the solution is being reviewed. (probably as a PR) Type:Bug Something is wrong or broken. This issue or PR is related to a bug in code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] A multi-period investment model allows generation from unavailable capacity
3 participants