Fix bug by allowing emission_scenarios to be None #46
Conversation
| @@ -54,10 +54,10 @@ def __init__( | |||
| ecs_mask_path=None, | |||
| ecs_mask_name=None, | |||
| base_period=(2001, 2010), | |||
| emission_scenarios=None, | |||
| emission_scenarios=(), | |||
There was a problem hiding this comment.
Just wondering if we want to be setting a default set of scenarios at all (then we wouldn't need to check for if == ()). Or if we should force the user to specify the scenarios, (can still be a default through the configs or something like that).
Perhaps a future adjustment to make.
There was a problem hiding this comment.
Hey @JMGilbert. Thanks for looking at this. Interesting bug. This is starting to feel more hacky as I'm not entirely clear on the meaning when emission_scenarios is None. It was kinda unclear to begin with so I don't mean it's not just your changes here. I have an in-line comment on updating the docstr so we at least know what the None means.
| @@ -37,7 +37,7 @@ class Climate: | |||
| Period for rebasing FAIR temperature anomalies. This should match the CIL projection system's base period. | |||
| emission_scenarios: list or None, optional | |||
| List of emission scenarios for which SCC will be calculated. Default | |||
| is ["ssp119", "ssp126", "ssp245", "ssp460", "ssp370", "ssp585"]. | |||
| is (), which gets set to ["ssp119", "ssp126", "ssp245", "ssp460", "ssp370", "ssp585"]. | |||
There was a problem hiding this comment.
@JMGilbert What does "None" actually mean then? This isn't documented anywhere (or it appears unclear).
|
@JMGilbert I'd recommend adding one sentence to |
Fix #45 by allowing
emission_scenariosto beNonewithout modifying its value during instantiation. This allows dscim-epa to replicate prior results and should allow for the update_dscim branch indscim-epato be merged.