Make parameters node accessible through the Web API #694

fpagnoux · 2018-07-24T20:13:53Z

Fixes #678 (See discussions there about route spec)

Use / rather than . in the path to access a parameter:
- For instance /parameter/benefits.basic_income becomes /parameter/benefits/basic_income
- Using . is for now still supported, but is considered deprecated and will be turned to a 301 redirection in the next major version.
Expose parameters metadata and source in the Web API and:

For instance, /parameter/benefits/basic_income contains:

{
  "description": "Amount of the basic income",
  "id": "benefits.basic_income",
  "metadata": {
    "reference": "https://law.gov.example/basic-income/amount",
    "unit": "currency-EUR"
  },
  "source": "https://github.com/openfisca/country-template/blob/3.2.2/openfisca_country_template/parameters/benefits/basic_income.yaml",
  "values": {
    "2015-12-01": 600.0
  }
}

Expose parameters nodes in the Web API
- For instance, /parameter/benefits now exists and contains:

{
  "description": "Social benefits",
  "id": "benefits",
  "source": "https://github.com/openfisca/country-template/blob/3.2.2/openfisca_country_template/parameters/benefits",
  "subparams": {
    "basic_income": {
      "description": "Amount of the basic income"
    },
    "housing_allowance": {
      "description": "Housing allowance amount (as a fraction of the rent)"
    }
  }
}

fpagnoux · 2018-07-31T16:42:56Z

Anyone for a review? @sandcha @maukoquiroga @Anna-Livia

fpagnoux · 2018-08-01T23:06:19Z

Question from @sandcha from slack (freely translated):

Isn't there some kind of inconsistency to expose nodes under parameter/ and not parameters/, while they represent a group of parameters and not a single parameter.

Yep, I thought about that aspect too. The distinction between parameters/ and parameter/ doesn't seem to be as clear now that the Web API expose nodes and reflects the tree structure of the parameters.

Exposing leafs under /parameter and nodes under /parameters would be a pain though. I can't know in advance if /benefits/basic_income is a leaf or a node, and I don't want to have to send two requests to figure it out.

The way I'd see the two routes is the following:

/parameter is a parametric route to explore the parameter graph. It exposes each nodes and leafs.
/parameters is a single "directory" route that exposes, in a flatten structure, basic information about all nodes and leafs.

I'm not fully satisfied with that, but I can't think of something better.

@MattiSG any light on the matter?

MattiSG · 2018-08-02T02:36:47Z

Bluntly because I'm short on time: I see a stable point in renaming /parameter to /parameters, using it as the single way to explore parameters, keeping plural even for accessing a single resource (authority argument 1, 2, 3), and renaming the current /parameters to something else, that would quite likely offer an overview of both parameters and variables in a single endpoint to reflect its only known use case (the legislation explorer).

fpagnoux · 2018-08-02T03:01:46Z

/overview 😄 ?

sandcha · 2018-08-01T21:14:10Z

CHANGELOG.md

+```
+
+* Expose parameters nodes in the Web API
+  - For instance, `/parameter/benefits` now exists and contains:


(Already said above) Shouldn't it be /parameters for a node as it's not a parameter? 🤔

sandcha · 2018-08-01T21:37:42Z

openfisca_web_api_preview/app.py

+        if parameter is None:
+            # Try legacy route
+            parameter_new_id = parameter_id.replace('.', '/')
+            parameter = data['parameters'].get(parameter_new_id)


Look for the parameter only when parameter_new_id differs from parameter_id to avoid looking into all parameters in that case (+2000 parameters in french model for example)?

We are not looking into all parameters. The get method of a dictionary has a O(1) average complexity as it uses hashes.

sandcha · 2018-08-01T22:13:28Z

tests/web_api/basic_case/__init__.py

@@ -1,9 +0,0 @@
-# -*- coding: utf-8 -*-


And delete tests/web_api/basic_case/ now empty directory?

Git only tracks files, not directory. If the directory is empty, it's not tracked by Git. So the directory you see on your machine is only local 🙂 .

sandcha · 2018-08-01T22:28:15Z

tests/web_api/test_parameters.py

+    assert_equal(parameter['values'], {u'2016-12-01': None, u'2010-01-01': 0.25})
+
+
+def test_bareme():


Rename to test_scale?

sandcha · 2018-08-01T23:00:32Z

tests/web_api/test_parameters.py

+        '/parameter/with%20url%20encoding': NOT_FOUND,
+        '/parameter/taxes.income_tax_rate/': OK,
+        '/parameter/taxes.income_tax_rate/too-much-nesting': NOT_FOUND,
+        '/parameter//taxes.income_tax_rate/': NOT_FOUND,


Update to/Add '/parameter//taxes/income_tax_rate/': NOT_FOUND, ?

sandcha · 2018-08-01T23:14:18Z

CHANGELOG.md

+}
+```
+
+* Expose parameters nodes in the Web API


Explicit that it exposes 1 level of depth only?

OK (see node below)

sandcha · 2018-08-02T07:53:58Z

openfisca_web_api_preview/loader/parameters.py

@@ -3,12 +3,12 @@
 from openfisca_core.parameters import Parameter, ParameterNode, Scale


-def transform_values_history(values_history):
-    values_history_transformed = {}
+def build_api_values_history(values_history):


sandcha · 2018-08-02T07:54:47Z

openfisca_web_api_preview/loader/parameters.py

-def walk_node(node, parameters, path_fragments):
+def build_source_url(absolute_file_path, country_package_metadata):
+    relative_path = absolute_file_path.replace(country_package_metadata['location'], '')
+    return u'{}/blob/{}{}'.format(


Use tree url rather than blob?

I think tree is for directories, and blob for files, so

either we detect whether the path is a file or a tree, and adjust the URL accordingly

or we just let GitHub redirect us when relevant, which is pretty transparent

I'm more in favor of 2.

Yes, in favour of 2.
I missed something in the definition of blob-jects. 😬'

sandcha · 2018-08-02T07:56:07Z

openfisca_web_api_preview/loader/parameters.py



-def walk_node(node, parameters, path_fragments):
+def build_source_url(absolute_file_path, country_package_metadata):


For taxes/income_tax_rate.yaml parameter, we get "source": "https://github.com/openfisca/openfisca-country-template/blob/3.2.2/openfisca_country_template/parameters/taxes/income_tax_rate.yaml" that leads to 404 error. Fix this?

Fixed by openfisca/country-template#50

sandcha · 2018-08-02T07:59:15Z

CHANGELOG.md

+
+```JSON
+{
+  "description": "Social benefits",


Explicit that a node might have metadata? 🤔

fpagnoux · 2018-08-02T20:26:44Z

Bluntly because I'm short on time: I see a stable point in renaming /parameter to /parameters, using it as the single way to explore parameters, keeping plural even for accessing a single resource (authority argument 1, 2, 3), and renaming the current /parameters to something else, that would quite likely offer an overview of both parameters and variables in a single endpoint to reflect its only known use case (the legislation explorer).

I agree with the target, but I'm wondering what would be a good strategy to go towards that while

not delaying too much the delivery of this feature
grouping Web API structural changes (Uniformise scales and parameters in the Web API #697 Expose value-level metadata for parameters in the Web API #696) into a single new major version

I think the most straight-forward strategy would be to

expose the nodes under /parameter as proposed by this PR
rename it to /parameters in v25 with the other changes

As much as it doesn't feel entirely satisfying to extend a route to rename it 2 months later, I think it won't be that bad for users, as they'll have to adapt their /parameter calls anyway.

fpagnoux force-pushed the expose-nodes branch from df70165 to 392fa9a Compare July 24, 2018 20:23

fpagnoux requested review from Anna-Livia, sandcha and bonjourmauko July 24, 2018 20:24

This was referenced Jul 24, 2018

Redirect /parameter/path.to.param to parameter/path/to/param #695

Closed

Uniformise scales and parameters in the Web API #697

Closed

bonjourmauko added the flow:team:doing label Jul 30, 2018

bonjourmauko assigned fpagnoux Jul 30, 2018

sandcha requested changes Aug 2, 2018

View reviewed changes

fpagnoux requested a review from sandcha August 2, 2018 20:27

sandcha approved these changes Aug 3, 2018

View reviewed changes

fpagnoux added 2 commits August 3, 2018 12:24

Expose parameters metadata in the Web API

0d1f471

Expose parameters node in the Web API

d5de9b6

fpagnoux force-pushed the expose-nodes branch from 2fed63c to dc11f34 Compare August 3, 2018 16:54

fpagnoux added 9 commits August 3, 2018 12:55

Expose parameter sources

b376e1b

Don't redondantly expose children ids

5e6c3de

Expose metadata

619c64e

Update spec

7b5b329

Rename children to subparams

78ed2fa

Use '/' instead of '.' to access parameters

abc75ba

Unburry tests

2fb0827

Bump version number

9d5fd6f

Always include metadata in parameters

608d561

fpagnoux force-pushed the expose-nodes branch from dc11f34 to 608d561 Compare August 3, 2018 16:55

fpagnoux merged commit ae9ffdb into master Aug 3, 2018

fpagnoux deleted the expose-nodes branch August 3, 2018 16:57

fpagnoux removed the flow:team:doing label Aug 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make parameters node accessible through the Web API #694

Make parameters node accessible through the Web API #694

fpagnoux commented Jul 24, 2018 •

edited

Loading

fpagnoux commented Jul 31, 2018

fpagnoux commented Aug 1, 2018 •

edited

Loading

MattiSG commented Aug 2, 2018

fpagnoux commented Aug 2, 2018

sandcha Aug 1, 2018 •

edited

Loading

sandcha Aug 1, 2018

fpagnoux Aug 2, 2018 •

edited by Morendil

Loading

sandcha Aug 1, 2018

fpagnoux Aug 2, 2018

sandcha Aug 1, 2018

sandcha Aug 1, 2018 •

edited

Loading

sandcha Aug 1, 2018

fpagnoux Aug 2, 2018

sandcha Aug 2, 2018

sandcha Aug 2, 2018

fpagnoux Aug 2, 2018

sandcha Aug 3, 2018

sandcha Aug 2, 2018

fpagnoux Aug 2, 2018

sandcha Aug 2, 2018

fpagnoux commented Aug 2, 2018 •

edited

Loading

		assert_equal(parameter['values'], {u'2016-12-01': None, u'2010-01-01': 0.25})


		def test_bareme():



		def walk_node(node, parameters, path_fragments):
		def build_source_url(absolute_file_path, country_package_metadata):

Make parameters node accessible through the Web API #694

Make parameters node accessible through the Web API #694

Conversation

fpagnoux commented Jul 24, 2018 • edited Loading

fpagnoux commented Jul 31, 2018

fpagnoux commented Aug 1, 2018 • edited Loading

MattiSG commented Aug 2, 2018

fpagnoux commented Aug 2, 2018

sandcha Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fpagnoux Aug 2, 2018 • edited by Morendil Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandcha Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fpagnoux commented Aug 2, 2018 • edited Loading

fpagnoux commented Jul 24, 2018 •

edited

Loading

fpagnoux commented Aug 1, 2018 •

edited

Loading

sandcha Aug 1, 2018 •

edited

Loading

fpagnoux Aug 2, 2018 •

edited by Morendil

Loading

sandcha Aug 1, 2018 •

edited

Loading

fpagnoux commented Aug 2, 2018 •

edited

Loading