Skip to content
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

Small changes to get it working on latest version of python and packages #1183

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EmanueleCannizzaro
Copy link

Thanks for contributing to OpenFisca! Remove this line, as well as any other in the following that don't fit your contribution :)

Breaking changes

  • In some module:
    • Remove…

New features

  • Introduce some_function()
    • Allows for…

Deprecations

  • Deprecate some_function.
    • The functionality is now provided by…

Technical changes

  • Rename private_function.

@MattiSG
Copy link
Member

MattiSG commented Jun 3, 2023

Thanks @EmanueleCannizzaro for your contribution!

Could you please correct the description of your pull request, and tell us the main differences of this changeset with #1181, #1161 and #1168? 🙂 Thanks!


from openfisca_core import parameters, periods
from openfisca_core.errors import ParameterParsingError
from openfisca_core.parameters import config


def contains_nan(vector):
print(vector)
Copy link
Member

Choose a reason for hiding this comment

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

Could you please remove this print statement?

@@ -2,17 +2,24 @@
import traceback

import numpy
import pandas as pd
Copy link
Member

Choose a reason for hiding this comment

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

If it is useful to test pandas here, I suggest we move this as an actual test, and that we require pandas optionally in case we want to run that specific test.

@@ -96,7 +96,8 @@ def calculate(self, variable_name: str, period):
"""Calculate ``variable_name`` for ``period``."""

if period is not None and not isinstance(period, Period):
period = periods.period(period)
##period = periods.period(period)
period = Period(period)
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change (the badly named periods.period function is actually a builder function.

from nptyping import types, NDArray as Array
# Modified by Emanuele Cannizzaro on 03/06/2023
#from typing import types, NDArray as Array
import types
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this! We should completely get rid of nptyping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants