Skip to content

Remove unused time_trend extrapolation cruft from utils.model_outputs(), update docstrs#53

Merged
kemccusker merged 16 commits into
dscim-v0.4.0from
clean_model_outputs
Apr 26, 2023
Merged

Remove unused time_trend extrapolation cruft from utils.model_outputs(), update docstrs#53
kemccusker merged 16 commits into
dscim-v0.4.0from
clean_model_outputs

Conversation

@brews
Copy link
Copy Markdown
Member

@brews brews commented Sep 28, 2022

Removes code relating to "time_trend" option to "extrapolation" in dscim.utils.utils.model_outputs().

Code is removed along with unused internal variables. Because they are unused, the following parameters are also removed from dscim.utils.utils.model_outputs():

  • extrap_formula
  • extrap_year
  • year_end_pred
  • base_year

Beware this will break code that tries to pass in these parameters.

Close #15

Remove code relating to "time_trends" extrapolation -- which
apparently has an incomplete/incorrect implementation here.
Removes additional cruft relating to crufty "time_trends" extrapolation.
@brews brews added the documentation Improvements or additions to documentation label Sep 28, 2022
@brews brews self-assigned this Sep 28, 2022
@brews brews requested a review from kemccusker September 28, 2022 20:51
@brews
Copy link
Copy Markdown
Member Author

brews commented Sep 28, 2022

Looks like CI tests are failing because they're using the old model_outputs() signature. I think I can fix this.

@brews brews changed the title Remove unused time_trend extrapolation cruft from utils.model_outputs(), update docstrs Draft: Remove unused time_trend extrapolation cruft from utils.model_outputs(), update docstrs Sep 28, 2022
@brews
Copy link
Copy Markdown
Member Author

brews commented Sep 28, 2022

Okay, things are working now. I think this is set for review.

@brews brews changed the title Draft: Remove unused time_trend extrapolation cruft from utils.model_outputs(), update docstrs Remove unused time_trend extrapolation cruft from utils.model_outputs(), update docstrs Sep 28, 2022
Comment thread src/dscim/utils/utils.py
global_c : xr.DataArray
Array with global consumption extrapolated to 2300. This is only used
when ``extrapolation_type`` is ``global_c_ratio``.
fix_global_c : int
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fix_global_c is relevant to the extrapolation method that we do use. Is it not used in the function?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@kemccusker There literally was no fix_global_c argument to this function, thus me removing it from the docstr. It was referencing something that wasn't there...

However, there is a fix_global_c variable here.

@brews brews assigned kemccusker and unassigned brews Apr 26, 2023
This reverts commit 657c12c, reversing
changes made to 86a6acc.
@JMGilbert JMGilbert changed the base branch from main to dscim-v0.4.0 April 26, 2023 19:41
@JMGilbert JMGilbert changed the base branch from dscim-v0.4.0 to main April 26, 2023 19:46
@JMGilbert JMGilbert changed the base branch from main to dscim-v0.4.0 April 26, 2023 19:46
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 26, 2023

Codecov Report

Merging #53 (3729b05) into dscim-v0.4.0 (096042f) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@               Coverage Diff                @@
##           dscim-v0.4.0      #53      +/-   ##
================================================
- Coverage         55.87%   55.81%   -0.06%     
================================================
  Files                17       17              
  Lines              1865     1856       -9     
================================================
- Hits               1042     1036       -6     
+ Misses              823      820       -3     
Impacted Files Coverage Δ
src/dscim/menu/main_recipe.py 69.75% <ø> (ø)
src/dscim/utils/utils.py 77.19% <100.00%> (+0.52%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kemccusker kemccusker merged commit 802f129 into dscim-v0.4.0 Apr 26, 2023
@kemccusker kemccusker deleted the clean_model_outputs branch April 26, 2023 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Large code block in dscim.utils.utils.model_outputs will never run

3 participants