Conversation
4094ef1 to
d40bce1
Compare
Could you take a look now? |
benjello
left a comment
There was a problem hiding this comment.
Not really qualified for this PR.
I just noticed something weird.
You may need @benoit-cty or @sandcha reviewing it.
Might also be nice if changes made here are propagated to france (or vice versa) to keep the tooling in line.
openfisca_extension_template/parameters/local_town/child_allowance/index.yaml
Outdated
Show resolved
Hide resolved
openfisca_extension_template/parameters/local_town/child_allowance/amount.yaml
Outdated
Show resolved
Hide resolved
What is weird?
I'm trying (since 2021) to equalise tooling of core/country/extension. Happy to help with France, it comes right after in the pipe. |
The yamlint addition |
I will make my point clearer : @benoit-cty and @sandcha worked a lot on the tooling in france (and I don't know if ti was portted to country-template and extensions) so I just warned about divergences and double work. |
Ah. I did add the yaml check as it is run in country-template. Maybe do we have a yamllint config in core or france? |
|
In OpenFisca-France, we did have this config in But I can't say much about it, I never work on it. |
My comment where from 2 years ago. Why ? That being said, we are using Poetry in all our Python project at LexImpact so it's easier to get support than on Hatch or UV... |
|
Sorry I didn't properly reply in my earlier comment. I'll do below:
There has been scattered discussions mainly in core. The last general discussion on roadmap is this one: openfisca/openfisca-core#942 The last council decision (RFC) on tooling is this one: openfisca/openfisca-core#1040 (comment) And the general problem this PR aims to fix is about properly testing against built versions openfisca/openfisca-core#1064
In particular, this PR relies on the virtuous interaction of I could still achieve the same result, although more laboriously —which we want to avoid, as these things have to be maintained over time— without So, it that is a blocker, I can remove it (as, again, at least here, is only accessory).
I have not knowledge on any of these tools. However, I'm personally agnostic on the tooling given that:
If we adopt a tool replacing make (which is ok, as seen in the RFC), the big feature it has to have is to allow for easy parallelisation of tasks. As
And that is a good point. So long as any tool uses a more or less standard pyproject.toml, adopting one tool or the other, at least for the use case of this PR, is an easily revertable or revokable decision. All that being said, I'd happily take a look at Thoughts? |
benoit-cty
left a comment
There was a problem hiding this comment.
If I install with make install, then do make format I end up with:
/bin/sh: 1: black: not found
I think it's a blocker but maybe I'm missing a command ?
I suggest to put poetry run in front of all command : dad766d
Why don't use poetry env remove --all in make uninstall instead of pip ?
Thanks for the details ! As you say, using Poetry is a nice step. It doesn't prevent using another tool in the future. It's not blocking at all 👍 If you are curious about Hatch, I use it for https://github.com/mlco2/codecarbon/blob/master/CONTRIBUTING.md#some-hatch-commands where it replace venv, poetry, make and tox. But it can be a huge step for the OpenFisca community. |
I'll try that. |
|
Ping @benoit-cty |
And due to some overly-clever ways that core handle things, I always fear that big changes will break things.
Thanks! That being said, the builder protocol is now standard, so we can perfectly mix together poetry and hatch. In another project, where I have to compile some extensions in C and C++, I use poetry + mason, for example. |
Part of openfisca/openfisca-core#1064
Depends on openfisca/country-template#159
poetryto ensure deterministic buildstoxto test builds in isolation