search

arfc / transition-scenarios

A repository to hold transition scenarios with Cyclus.
Other
3 stars 12 forks source link

Start year flexibility #167

Closed RhysMacMillan closed 2 months ago

RhysMacMillan commented 2 months ago

This PR adds the ability for users to choose their own first year for data frame columns. The year 1965 occurs as an arbitrary start year throughout the scripts folder, and this change allows more flexibility for the user.

Edit: This PR addresses part of issue #60, specifically the hard coded variables in dataframe_analysis_changes

pep8speaks commented 2 months ago

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

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2024-08-13 19:40:04 UTC
RhysMacMillan commented 2 months ago

Thank you for the feedback @nsryan2. I couldn't decide between y0 and s-y but I do like y0 more than start_year because of its brevity. I would like to change this variable name, the other assumptions in dataframe_analysis and the PEP 8 style issue in one action, so would making a new PR referencing this one be the best course of action?

nsryan2 commented 2 months ago

Ok! Yeah, I think y0 is better than s_y.

You can make changes by making changes on your clone and then pushing them to the dataframe_analysis_changes branch you made this PR from on your fork. You don't need to make a new PR.

Edit: I have updated my suggestions to say y0 instead of start_year. https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request

nsryan2 commented 2 months ago

@RhysMacMillan are you still adding changes to this PR? You mentioned some other assumptions above but I'm not sure if you've done all the ones you wanted to do. Otherwise, it looks good to me

RhysMacMillan commented 2 months ago

Yes @nsryan2 I have one last change for the start year assumption. In create_cyclus_input.py there's a global variable; start_year = 1965 and I'm not sure how to address this change. My first thought was to have a user input but I don't know how the scripts are interacted with so I didn't want to make it cumbersome.

nsryan2 commented 2 months ago

This would be a great thing to mention in #162! If you make a note of that change as a comment in that issue, I think that would be good. If that's the case, I'll merge this PR. Thanks for the contribution!