PolicyEngine / policyengine-core

Core microsimulation engine for PolicyEngine models. Forked from OpenFisca-Core.
https://policyengine.github.io/policyengine-core
GNU Affero General Public License v3.0
14 stars 19 forks source link

Uncap nptyping #185

Closed MaxGhenis closed 1 month ago

MaxGhenis commented 4 months ago

Currently <2:

https://github.com/PolicyEngine/policyengine-core/blob/4c67661f0e7e23d526f5bc99af91f8b54a9bd0c9/setup.py#L22

But it's now on 2.5.0: https://pypi.org/project/nptyping/

tawandamoyo commented 2 months ago

Hello @MaxGhenis , I have run into some difficulties.

Initially it looked like using nptyping without version constraints worked but this was due to the cached old version.

When I completely uninstalled nptyping and reinstalled dependencies with an uncapped nptyping, the tests failed with the following error:

ImportError: cannot import name 'types' from 'nptyping'

This error persists with any version of nptyping >= 2.0.0.

I'm not entirely sure but it appears that there may have been a breaking change in nptyping from version 2 onwards, that leads to an error here.

I'd really appreciate your insights on how to proceed.

Thanks

MaxGhenis commented 2 months ago

That link doesn't work. Can you check the nptyping release notes and other parts of the package to see how we should migrate our code?

MaxGhenis commented 2 months ago

I also wonder if we should just migrate to np.typing, see https://github.com/ramonhagenaars/nptyping/discussions/122

I don't think this was available when it was first integrated to openfisca/policyengine

@nikhilwoodruff wdyt?

tawandamoyo commented 2 months ago

That link doesn't work. Can you check the nptyping release notes and other parts of the package to see how we should migrate our code?

Alright, looking into it (and fixed the link)

tawandamoyo commented 1 month ago

@MaxGhenis so should I use np.typing?

MaxGhenis commented 1 month ago

If it provides the same functionality, np.typing will be better maintained so yes. But might be a heavier lift.