Skip to content

Upgrade to Python 3.13 #338

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

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,22 @@ jobs:
strategy:
matrix:
os: [ ubuntu-latest, windows-latest ]
python-version: [ "3.12", "3.13" ]
fail-fast: false
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repo
uses: actions/checkout@v3
- name: Set up Python
- name: Enable long paths in Git (Windows Only)
if: runner.os == 'Windows'
run: git config --system core.longpaths true
- name: Enable Win32 long paths via registry (Windows Only)
if: runner.os == 'Windows'
run: reg add "HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\FileSystem" /v LongPathsEnabled /t REG_DWORD /d 1 /f
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: "3.12"
python-version: ${{ matrix.python-version }}
- name: Install package
run: make install
- name: Run tests
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ format:

install:
pip install -e ".[dev]" --config-settings editable_mode=compat
pip install policyengine-us
pip install git+https://github.com/noman404/policyengine-us.git@noman404/python3.13
Copy link
Contributor

Choose a reason for hiding this comment

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

issue, blocking: This ties us to an installation we do not want

Copy link
Contributor

Choose a reason for hiding this comment

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

question: @nikhilwoodruff How did we resolve the overlapping deps issue Al raised before? And is it still present?

pip install policyengine-uk

test-country-template:
Expand Down
5 changes: 5 additions & 0 deletions changelog_entry.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- bump: major
Copy link
Contributor

Choose a reason for hiding this comment

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

is this backwards-incompatible?

Copy link
Contributor Author

@noman404 noman404 Feb 5, 2025

Choose a reason for hiding this comment

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

Python codeabase should be fine, cause there is no change related to python language specific code. However numpy is not compatible, there are several changes in data type like, float64, int64, inf, default type definition etc.

https://numpy.org/devdocs/numpy_2_0_migration_guide.html

changes:
changed:
- python 3.13.0
- numpy 2.1.0
5 changes: 5 additions & 0 deletions policyengine_core/commons/formulas.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ def concat(this: ArrayLike[str], that: ArrayLike[str]) -> ArrayType[str]:
array(['this1.0', 'that2.5']...)

"""
if isinstance(this, tuple):
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Why can't these be tuples?

raise TypeError("First argument must not be a tuple.")

if isinstance(that, tuple):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, non-blocking: Can't we just check both of these args and have one message for both?

raise TypeError("Second argument must not be a tuple.")

if isinstance(this, numpy.ndarray) and not numpy.issubdtype(
this.dtype, numpy.str_
Expand Down
3 changes: 2 additions & 1 deletion policyengine_core/enums/enum_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ def decode_to_str(self) -> numpy.str_:
"""
return numpy.select(
[self == item.index for item in self.possible_values],
[item.name for item in self.possible_values],
[str(item.name) for item in self.possible_values],
default="unknown",
)

def __repr__(self) -> str:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,15 @@ def __getitem__(self, key: str) -> Any:
enum = type(key[0])
key = numpy.select(
[key == item for item in enum],
[item.name for item in enum],
[str(item.name) for item in enum],
default="unknown",
)
elif isinstance(key, EnumArray):
enum = key.possible_values
key = numpy.select(
[key == item.index for item in enum],
[item.name for item in enum],
default="unknown",
)
else:
key = key.astype("str")
Expand Down
4 changes: 2 additions & 2 deletions policyengine_core/populations/group_population.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def max(self, array: ArrayLike, role: Role = None) -> ArrayLike:
return self.reduce(
array,
reducer=numpy.maximum,
neutral_element=-numpy.infty,
neutral_element=-numpy.inf,
role=role,
)

Expand All @@ -256,7 +256,7 @@ def min(self, array: ArrayLike, role: Role = None) -> ArrayLike:
return self.reduce(
array,
reducer=numpy.minimum,
neutral_element=numpy.infty,
neutral_element=numpy.inf,
role=role,
)

Expand Down
6 changes: 5 additions & 1 deletion policyengine_core/populations/population.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,11 @@ def get_rank(
# We double-argsort all lines of the matrix.
# Double-argsorting gets the rank of each value once sorted
# For instance, if x = [3,1,6,4,0], y = numpy.argsort(x) is [4, 1, 0, 3, 2] (because the value with index 4 is the smallest one, the value with index 1 the second smallest, etc.) and z = numpy.argsort(y) is [2, 1, 4, 3, 0], the rank of each value.
sorted_matrix = numpy.argsort(numpy.argsort(matrix))

# because of the infinities the first sort creates positional indices
# The second argsort converts these positions to ranks, thus fixes the broken sort issue
first_argsort = numpy.argsort(matrix, axis=1, kind="stable")
sorted_matrix = numpy.argsort(first_argsort, axis=1, kind="stable")
Copy link
Contributor

Choose a reason for hiding this comment

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

blocking: I'd love @nikhilwoodruff to confirm the logic here, as I'm less familiar with numpy


# Build the result vector by taking for each person the value in the right line (corresponding to its household id) and the right column (corresponding to its position)
result = sorted_matrix[ids, positions]
Expand Down
8 changes: 4 additions & 4 deletions policyengine_core/taxscales/marginal_rate_tax_scale.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ def calc(
#
# numpy.finfo(float_).eps
thresholds1 = numpy.outer(
factor + numpy.finfo(numpy.float_).eps,
factor + numpy.finfo(numpy.float64).eps,
Copy link
Contributor

Choose a reason for hiding this comment

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

numpy.array(self.thresholds + [numpy.inf]),
)

if round_base_decimals is not None:
thresholds1 = numpy.round_(thresholds1, round_base_decimals)
thresholds1 = numpy.round(thresholds1, round_base_decimals)

a = numpy.maximum(
numpy.minimum(base1, thresholds1[:, 1:]) - thresholds1[:, :-1], 0
Expand All @@ -82,8 +82,8 @@ def calc(

else:
r = numpy.tile(self.rates, (len(tax_base), 1))
b = numpy.round_(a, round_base_decimals)
return numpy.round_(r * b, round_base_decimals).sum(axis=1)
b = numpy.round(a, round_base_decimals)
return numpy.round(r * b, round_base_decimals).sum(axis=1)

def combine_bracket(
self,
Expand Down
4 changes: 2 additions & 2 deletions policyengine_core/taxscales/rate_tax_scale_like.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,12 @@ def bracket_indices(
#
# numpy.finfo(float_).eps
thresholds1 = numpy.outer(
+factor + numpy.finfo(numpy.float_).eps,
+factor + numpy.finfo(numpy.float64).eps,
numpy.array(self.thresholds),
)

if round_decimals is not None:
thresholds1 = numpy.round_(thresholds1, round_decimals)
thresholds1 = numpy.round(thresholds1, round_decimals)

return (base1 - thresholds1 >= 0).sum(axis=1) - 1

Expand Down
3 changes: 2 additions & 1 deletion policyengine_core/tools/simulation_dumper.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ def _dump_entity(population, directory):
else:
encoded_roles = np.select(
[population.members_role == role for role in flattened_roles],
[role.key for role in flattened_roles],
[str(role.key) for role in flattened_roles],
default="unknown",
)
np.save(os.path.join(path, "members_role.npy"), encoded_roles)

Expand Down
4 changes: 3 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

general_requirements = [
"pytest>=8,<9",
"numpy~=1.26.4",
"numpy~=2.1.0",
"sortedcontainers<3",
"numexpr<3",
"dpath<3",
Expand All @@ -25,6 +25,7 @@
"pyvis>=0.3.2",
"microdf_python>=0.4.3",
"huggingface_hub>=0.25.1",
"standard-imghdr",
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Why do we need this package? I see no reference to it in the PR changes

]

dev_requirements = [
Expand Down Expand Up @@ -60,6 +61,7 @@
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
"Programming Language :: Python :: 3.13",
"Topic :: Scientific/Engineering :: Information Analysis",
],
description="Core microsimulation engine enabling country-specific policy models.",
Expand Down
10 changes: 10 additions & 0 deletions tests/core/commons/test_formulas.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,13 @@ def test_switch_when_values_are_empty():

with pytest.raises(AssertionError):
assert commons.switch(conditions, value_by_condition)


def test_concat_tuple_inputs():
with pytest.raises(TypeError, match="First argument must not be a tuple."):
commons.concat(("a", "b"), numpy.array(["c", "d"]))

with pytest.raises(
TypeError, match="Second argument must not be a tuple."
):
commons.concat(numpy.array(["a", "b"]), ("c", "d"))
7 changes: 6 additions & 1 deletion tests/core/parameters_fancy_indexing/test_fancy_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,10 @@ class TypesZone(Enum):
z1 = "Zone 1"
z2 = "Zone 2"

zone = np.asarray([TypesZone.z1, TypesZone.z2, TypesZone.z2, TypesZone.z1])
zone = np.asarray(
[
Copy link
Contributor

Choose a reason for hiding this comment

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

question: Why does this behavior change?

z.name
for z in [TypesZone.z1, TypesZone.z2, TypesZone.z2, TypesZone.z1]
]
)
assert_near(P.single.owner[zone], [100, 200, 200, 100])
3 changes: 2 additions & 1 deletion tests/core/test_tracers.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,8 @@ def test_log_aggregate(tracer):

lines = tracer.computation_log.lines(aggregate=True)
assert (
lines[0] == " A<2017, (default)> = {'avg': 1.0, 'max': 1, 'min': 1}"
lines[0].strip()
== "A<2017, (default)> = {'avg': np.float64(1.0), 'max': np.int64(1), 'min': np.int64(1)}"
)


Expand Down