Skip to content

Allow array parameters #982

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Mar 23, 2021
Merged

Allow array parameters #982

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d363c82
Improve invalid parameter type error message
MattiSG Mar 17, 2021
315e2c6
Allow arrays in parameters
MattiSG Mar 17, 2021
ec93ac8
Bump version to 35.2.0
MattiSG Mar 23, 2021
4b36f6f
Unify brackets
MattiSG Mar 23, 2021
8a23aae
Fix typo
MattiSG Mar 23, 2021
9d8f6ce
Fix indentation
MattiSG Mar 23, 2021
4e66961
Document arrays and booleans as parameter values
MattiSG Mar 23, 2021
4faafff
Fix title hierarchy
MattiSG Mar 23, 2021
77d5808
Unify import styles
MattiSG Mar 23, 2021
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

### 35.2.0 [#982](https://github.com/openfisca/openfisca-core/pull/982)

#### Technical changes

- Allow parameters to be arrays.

### 35.1.1 [#981](https://github.com/openfisca/openfisca-core/pull/981)

#### Technical changes
Expand Down
7 changes: 4 additions & 3 deletions openfisca_core/parameters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import os
import sys
import traceback
import typing

import yaml
import numpy as np
Expand All @@ -36,7 +37,7 @@
# 'unit' and 'reference' are only listed here for backward compatibility.
# It is now recommended to include them in metadata, until a common consensus emerges.
COMMON_KEYS = {'description', 'metadata', 'unit', 'reference', 'documentation'}
ALLOWED_PARAM_TYPES = (float, int, bool, type(None))
ALLOWED_PARAM_TYPES = (float, int, bool, type(None), typing.List)
Copy link
Member

Choose a reason for hiding this comment

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

Any preference between typing.List and just List? Asking just for consistency (current usage is from typing import XXX, but that is not consistent with other libraries usage, for example numpy and so on).

Copy link
Member Author

Choose a reason for hiding this comment

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

No preference.

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 use from typing import List just for consistency? Otherwise GTM :)



def date_constructor(loader, node):
Expand Down Expand Up @@ -319,7 +320,7 @@ def validate(self, data):
)
if not isinstance(value, ALLOWED_PARAM_TYPES):
raise ParameterParsingError(
"Invalid value in {} : {}".format(self.name, value),
"Value in {} has type {}, which is not one of the allowed types ({}): {}".format(self.name, type(value), ALLOWED_PARAM_TYPES, value),
self.file_path
)

Expand Down Expand Up @@ -851,7 +852,7 @@ def load_parameter_file(file_path, name = ''):
:returns: An instance of :any:`ParameterNode` or :any:`Scale` or :any:`Parameter`.
"""
if not os.path.exists(file_path):
raise ValueError("{} doest not exist".format(file_path))
raise ValueError("{} does not exist".format(file_path))
if os.path.isdir(file_path):
return ParameterNode(name, directory_path = file_path)
data = _load_yaml_file(file_path)
Expand Down
81 changes: 41 additions & 40 deletions openfisca_web_api/openAPI.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,49 +34,49 @@ paths:
post:
summary: "Run a simulation"
tags:
- Calculations
- Calculations
operationId: "calculate"
consumes:
- "application/json"
- "application/json"
produces:
- "application/json"
- "application/json"
parameters:
- in: "body"
name: "Situation"
description: 'Describe the situation (persons and entities). Add the variable you wish to calculate in the proper entity, with null as the value. Learn more in our official documentation: https://openfisca.org/doc/openfisca-web-api/input-output-data.html'
description: "Describe the situation (persons and entities). Add the variable you wish to calculate in the proper entity, with null as the value. Learn more in our official documentation: https://openfisca.org/doc/openfisca-web-api/input-output-data.html"
required: true
schema:
$ref: '#/definitions/SituationInput'
$ref: "#/definitions/SituationInput"
responses:
200:
description: "The calculation result is sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
schema:
$ref: '#/definitions/SituationOutput'
$ref: "#/definitions/SituationOutput"
404:
description: "A variable mentioned in the input situation does not exist in the loaded tax and benefit system. Details are sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
400:
description: "The request is invalid. Details about the error are sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
/parameters:
get:
tags:
- "Parameters"
- "Parameters"
summary: "List all available parameters"
operationId: "getParameters"
produces:
- "application/json"
- "application/json"
responses:
200:
description: "The list of parameters is sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
schema:
$ref: '#/definitions/Parameters'
$ref: "#/definitions/Parameters"
/parameter/{parameterID}:
get:
tags:
Expand All @@ -95,13 +95,13 @@ paths:
200:
description: "The requested parameter's information is sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
schema:
$ref: "#/definitions/Parameter"
404:
description: "The requested parameter does not exist"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
/variables:
get:
tags:
Expand All @@ -114,7 +114,7 @@ paths:
200:
description: "The list of variables is sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
schema:
$ref: "#/definitions/Variables"
/variable/{variableID}:
Expand All @@ -135,13 +135,13 @@ paths:
200:
description: "The requested variable's information is sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
schema:
$ref: "#/definitions/Variable"
404:
description: "The requested variable does not exist"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
/entities:
get:
tags:
Expand All @@ -154,61 +154,61 @@ paths:
200:
description: "The list of the entities as well as their information is sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
schema:
$ref: "#/definitions/Entities"
/trace:
post:
summary: "Explore a simulation's steps in details."
tags:
- Calculations
- Calculations
operationId: "trace"
consumes:
- "application/json"
- "application/json"
produces:
- "application/json"
- "application/json"
parameters:
- in: "body"
name: "Situation"
description: 'Describe the situation (persons and entities). Add the variable you wish to calculate in the proper entity, with null as the value.'
description: "Describe the situation (persons and entities). Add the variable you wish to calculate in the proper entity, with null as the value."
required: true
schema:
$ref: '#/definitions/SituationInput'
$ref: "#/definitions/SituationInput"
responses:
200:
description: "The calculation details are sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
schema:
$ref: '#/definitions/Trace'
$ref: "#/definitions/Trace"
404:
description: "A variable mentioned in the input situation does not exist in the loaded tax and benefit system. Details are sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
400:
description: "The request is invalid. Details about the error are sent back in the response body"
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"
/spec:
get:
summary: Provide the API documentation in an OpenAPI format
tags:
- Documentation
- Documentation
operationId: spec
produces:
- application/json
- application/json
responses:
200:
description: The API documentation is sent back in the response body
headers:
$ref: '#/commons/Headers'
$ref: "#/commons/Headers"

definitions:
Parameter:
type: "object"
properties:
values:
$ref: '#/definitions/Values'
$ref: "#/definitions/Values"
brackets:
type: "object"
additionalProperties:
Expand All @@ -219,7 +219,7 @@ definitions:
type: "object"
properties:
definition:
type: 'string'
type: "string"
metadata:
type: "object"
description:
Expand All @@ -237,9 +237,9 @@ definitions:
type: "object"
properties:
description:
type: 'string'
type: "string"
href:
type: 'string'
type: "string"

Variable:
type: "object"
Expand Down Expand Up @@ -284,9 +284,9 @@ definitions:
type: "object"
properties:
description:
type: 'string'
type: "string"
href:
type: 'string'
type: "string"

Formula:
type: "object"
Expand All @@ -303,10 +303,11 @@ definitions:
format: "float"

Values:
description: All keys are ISO dates. Values can be numbers, booleans, or arrays of a single type (number, boolean or string).
type: "object"
additionalProperties:
type: "number"
format: "float"
additionalProperties: true
# propertyNames: # this keyword is part of JSON Schema but is not supported in OpenAPI Specification at the time of writing, see https://swagger.io/docs/specification/data-models/keywords/#unsupported
# pattern: "^[12][0-9]{3}-[01][0-9]-[0-3][0-9]$" # all keys are ISO dates

Entities:
type: "object"
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

setup(
name = 'OpenFisca-Core',
version = '35.1.1',
version = '35.2.0',
author = 'OpenFisca Team',
author_email = '[email protected]',
classifiers = [
Expand Down
6 changes: 6 additions & 0 deletions tests/core/parameter_validation/array_type.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
description: Array types should be allowed.
values:
2013-01-01:
value: [ FR ]
2018-06-01:
value: [ ES, FR, IT, NZ ]
11 changes: 8 additions & 3 deletions tests/core/parameter_validation/test_parameter_validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
year = 2016


def check(file_name, keywords):
def check_fails_with_message(file_name, keywords):
path = os.path.join(BASE_DIR, file_name) + '.yaml'
try:
load_parameter_file(path, file_name)
Expand All @@ -22,7 +22,7 @@ def check(file_name, keywords):
@pytest.mark.parametrize("test", [
('indentation', {'Invalid YAML', 'indentation.yaml', 'line 2', 'mapping values are not allowed'}),
('wrong_scale', {'Unexpected property', 'scale[1]', 'treshold'}),
('wrong_value', {'Invalid value', 'wrong_value[2015-12-01]', '1A'}),
('wrong_value', {'not one of the allowed types', 'wrong_value[2015-12-01]', '1A'}),
('unexpected_key_in_parameter', {'Unexpected property', 'unexpected_key'}),
('wrong_type_in_parameter', {'must be of type object'}),
('wrong_type_in_value_history', {'must be of type object'}),
Expand All @@ -37,7 +37,12 @@ def check(file_name, keywords):
])
def test_parsing_errors(test):
with pytest.raises(ParameterParsingError):
check(*test)
check_fails_with_message(*test)


def test_array_type():
path = os.path.join(BASE_DIR, 'array_type.yaml')
load_parameter_file(path, 'array_type')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
load_parameter_file(path, 'array_type')
assert load_parameter_file(path, 'array_type')

Copy link
Member

Choose a reason for hiding this comment

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

It will anyway pass as it is a truthy value -otherwise it would fail. Just a proposition for reader's clarity.



def test_filesystem_hierarchy():
Expand Down