Remove surv_rate parameter object
#886
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #886 +/- ##
==========================================
- Coverage 79.83% 79.82% -0.02%
==========================================
Files 18 18
Lines 4181 4183 +2
==========================================
+ Hits 3338 3339 +1
- Misses 843 844 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@jdebacker. I just added a PR to your branch that updates the version number in |
|
@jdebacker. I am running into an error when I run |
|
@rickecon I think this is a Python 3.11 thing. I posted a similar issue in the UN-OG-Training repo today. Will need to see if we can find a way to read these pickle files created in an earlier Python or if we need to recreate them all. |
Updated version in setup.py and ogcore/__init__.py
|
@jdebacker For now, maybe we should just pin to Python 3.10 ( |
|
@rickecon I wanted to look around at a solution to the unpickling issue since I would like to keep as loose of Python version pins as possible (being flexible here is important, for example, in some government bureaucracies where it takes many months to get new software versions approved). The issue affecting us here (not being able to read pickle files created in older versions of Python) only affects files created with
Let me try to recreate versions of these saved with |
|
@rickecon Tests are now passing, here's what I did with respect to the four files noted above:
Note, the reason I chose to bypass tests for cases (1) and (2) is because, at the moment, it's been difficult to generate new mono tax function estimates in OG-USA (see the notes in PR #73). |
rickecon
reviewed
Oct 16, 2023
| @@ -15,7 +15,7 @@ jobs: | |||
| strategy: | |||
| matrix: | |||
| os: [ubuntu-latest, macos-latest, windows-latest] | |||
| python-version: ["3.9", "3.10"] | |||
| python-version: ["3.9", "3.10", "3.11"] | |||
There was a problem hiding this comment.
@jdebacker. I would remove the Python 3.9 tests here if we are going to test Python 3.11. As it is written now, this is 9 sets of CI tests. I think we are going to bump into the limits of what GitHub is willing to compute. Plus, let's encourage people to use Python 3.10 or above implicitly here.
In summary, I think you should remove Python 3.9 in the name in line 1, and remove Python 3.9 in the version matrix in line 18.
|
@rickecon Thanks for your review. I'm going to merge this. I'm ok with the time on GH Actions (things run in parallel anyway) and really like ensuring compatibility with older versions of Python for the reasons expressed above. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the
surv_rateparameter object from OG-Core. This object is redundant with the mortality rates (rho).surv_rateis removed from the following files:default_parameters.jsonparameters.py, wheresurv_rateis used to calculate the SS population distribution in the case of constant demographics. Here, we replacesurv_ratewith1 - rho.testing_parameters.jsonPartially addresses Issue #885