Training upgrade #139

verbman · 2024-03-20T21:08:04Z

Technical improvement.
Major change
Impacted periods: all.
Impacted areas: all
Details:
- Improvements to bootstrap.sh to improve experience for first time users:
  - remove the need to set COUNTRY_NAME and REPOSITORY_URL before running.
  - provide a confirmation option/exit script option before changes are made
  - allow for hypens and spaces appropriately
  - set master branch to main by default
  - print to console the commit actions along with description
- Change CONTRIBUTING.md to allow for improved text replacements
  - Replace reference to master with main
- Change Makefile
  - Remove reference to OpenFisca France replace with new repository name
  - replace argument -e with longer form --editable to aid newcomers
  - add new formating commands for isort, pyupgrade and yamllint
- Author default pyproject.toml based on the older setup.py and setup.cfg
  - Set version to 7.0.0
  - Add in [project.urls]: Homepage, Repository, Documentation, Issues and Changelog
  - Add in [project.optional-dependencies]: new dependancies isort, pyupgrade and yamllint
  - Add [tool.pytest.ini_options] with filterwarnings set to "error"
  - Add [tool.pylint.messages_control] from setup.cfg but in long form for readability
  - Add [tool.isort] section
- Add .flake8 and .yamllint to project root as these projects do not support pyproject.toml format
- Alter README.md to allow for improved text replacements
  - Remove commands (line 21) for export COUNTRY_NAME=France and export REPOSITORY_URL=... due to bootstrap.sh improvements
  - Adjust (Line 28) from git push origin master to git push origin main
  - General changes to support change to bootstrap flow
- Remove setup.py and setup.cfg
- Update .github/*.sh and .github/workflows/workflow.yaml files to reference main instead of master
- Fix linting issues in:
  - openfisca_country_template/texts/social_security_contribution.yaml
  - openfisca_country_template/texts/situations/income_tax.yaml
  - openfisca_country_template/variables/demographics.py
  - openfisca_country_template/variables/taxes.py
- Alter CHANGELOG.md, add example entry and alter bootstrap.sh to strip out country-template entries. This resolves issue 116.

Note that this has been tested against OpenFisca-core and requires corresponding pull request to pass the OpenFisca-core test suite.

If this is approved, it is the intention to upgrade the documentation surrounding OpenFisca which will likely include the *.md files in this repository and result in a subsequent PR.

…plus improvements to bootstrap.sh

MattiSG

Looks promising! 👏

.flake8

.yamllint

CHANGELOG.md

Makefile

bootstrap.sh

pyproject.toml

MattiSG · 2024-03-28T06:19:56Z

Note: after merging, we'll need to switch the main branch from master to main.

MattiSG · 2024-03-28T06:24:42Z

Did you see #109? Could it be (have been) helpful?

…remote origin

… upgrade

…Planning

Co-authored-by: Matti Schneider <[email protected]>

MattiSG

The version number is not reinitialised after the bootstrap script is executed, meaning the first version number is 7.0.0.

I tried executing, and got the following:

test/country-template-training-upgrade [main]
› python --version
Python 3.11.7
test/country-template-training-upgrade [main]
› pip install openfisca-senegal
Collecting openfisca-senegal
  Downloading OpenFisca_Senegal-0.10.3-py3-none-any.whl.metadata (1.5 kB)
Collecting OpenFisca-Core<36.0,>=35.0.1 (from openfisca-senegal)
…
Downloading wcwidth-0.2.13-py2.py3-none-any.whl (34 kB)
Building wheels for collected packages: numpy
  Building wheel for numpy (pyproject.toml) ... error
  error: subprocess-exited-with-error
  
  × Building wheel for numpy (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [1264 lines of output]
      setup.py:66: RuntimeWarning: NumPy 1.20.3 may not yet support Python 3.11.

We insist that Python version should be 3.9, but give no guidance on how to install that specific version. We can consider that this is out of scope, but “should print "Python 3.9.xx"” is a bit light compared to “Install Python 3.9” with a link to how to install that version.

In my case (macOS), I could make it work by using brew install [email protected] and then using pip3.9 instead of pip.

As written in the README, I tried to run openfisca serve --port 5000 after installing, and that failed with openfisca: command not found. I used instead make serve-local which is suggested as an alternative, and it worked. If we cannot make openfisca serve work, let's remove that instruction.

MattiSG · 2024-04-13T08:40:41Z

CHANGELOG.md

+## 7.0.0 [#139](https://github.com/openfisca/country-template/pull/139)
+
+* Technical improvement.
+* Major change


I don't see any breaking change, so according to semver this could just be a minor 🙂

There is the breaking change discussed between this and OpenFisca-Core tests...

CHANGELOG.md

README.md

bootstrap.sh

pyproject.toml

MattiSG · 2024-04-13T08:56:49Z

bootstrap.sh

+	read JURISDICTION_NAME
+	if [[ "$JURISDICTION_NAME"  != "" ]]
+	then
+		lowercase_jurisdiction_name=$(echo $JURISDICTION_NAME | tr '[:upper:]' '[:lower:]')


If we want to make this even more forgiving, we should remove diacritics. For example, if I type Sénégal as a jurisdiction, this should convert to senegal, as otherwise I get:

› pip install openfisca-sénégal ERROR: Invalid requirement: 'openfisca-sénégal'

If this is too costly to implement, we probably should clarify “without accents” in the read prompt 🙂

Implemented and tested possible solution, please see commit #f38f8f9

MattiSG · 2024-04-13T09:00:25Z

bootstrap.sh

-echo '************'
-echo '* All set! *'
-echo '************'
+exec bash


This starts a bash session without loading .profile and other session-specific setup, which is very confusing.

Yes, I have not found another solution automatically orientating the user to the newly created directory. I'm open to suggestions on this. We can revert which leaves them at a bash prompt located in a non existent folder which might also be confusing.

Co-authored-by: Matti Schneider <[email protected]>

…ME.md improvements

MattiSG

Thank you for these changes!

I realised the current implementation removes the ability to use env variables, which prevents automated execution. We should instead ask interactively only if it is not readily available.

I'll take over the last changes 🙂

Exit with code 3 in case of user interruption

Example: Virgin Islands

The profile files are not loaded in this way

Remove the need to edit it

MattiSG

I made several changes, please double-check them before merging 🙂

The remote over HTTPS needs constant authentication and user would need to remove and then re-add the remote

Was silent in https://github.com/openfisca/country-template/actions/runs/8810580013, which led to failed merge in #139

verbman and others added 4 commits March 17, 2024 17:46

Major proposed update to country template, introduces pyproject.toml …

9f0b08c

…plus improvements to bootstrap.sh

Allow for versions of python >= 3.9

0ba2296

Adjustment to allow OpenFisca-core tests to find repository

49726ba

Update CHANGELOG.md

8c54f5e

verbman mentioned this pull request Mar 20, 2024

Add repository URL to pyproject.toml #138

Closed

Resolution for issue 116 regarding changelog entries

08141e8

verbman force-pushed the training-upgrade branch from 4707858 to 08141e8 Compare March 21, 2024 07:24

MattiSG requested changes Mar 28, 2024

View reviewed changes

First response to review, quiet git commits in bootstrap.sh, not add …

d761ec5

…remote origin

verbman mentioned this pull request Apr 4, 2024

Use cookiecutter for bootstrapping #109

Closed

verbman added 3 commits April 8, 2024 21:36

Bootstrap.sh and readme.md updates to improve flow for first users.

600f986

Disable line-length rule in yamllint, update README.md to include pip…

d96b2f3

… upgrade

Update pyproject.toml on install to switch from Production/Stable to …

b7b130f

…Planning

verbman requested a review from MattiSG April 8, 2024 10:22

verbman and others added 3 commits April 8, 2024 22:56

CHANGELOG.md updates

d0c1ffc

Apply suggestions from code review

4db6df3

Co-authored-by: Matti Schneider <[email protected]>

Snakecase and while improvements to bootstrap

ef71696

MattiSG requested changes Apr 13, 2024

View reviewed changes

verbman and others added 2 commits April 22, 2024 14:18

Apply suggestions from code review

0d309ed

Co-authored-by: Matti Schneider <[email protected]>

Possible diacritics resolution, simplified bootstrap.sh file and READ…

f38f8f9

…ME.md improvements

MattiSG requested changes Apr 23, 2024

View reviewed changes

MattiSG added 7 commits April 23, 2024 16:40

Support env variables in bootstrap.sh

188ecd5

Exit with code 3 in case of user interruption

Bring backwards compatibility to env variables

875d55d

Fix casing and improve phrasings

061900c

Support spaces in jurisdiction name

7cdc8bc

Example: Virgin Islands

Remove unnecessary options

3fe43ce

Do not create a new shell session on bootstrap

de24fb4

The profile files are not loaded in this way

Fix whitespace

ae5842a

MattiSG added 4 commits April 23, 2024 16:40

Unify conditions syntax

363ecd0

Automatically set up remote

40819d5

Make example changelog entry directly usable

225d206

Remove the need to edit it

Simplify changelog entry

3456cb6

MattiSG approved these changes Apr 23, 2024

View reviewed changes

verbman and others added 4 commits April 24, 2024 12:12

Remove unneeded command from bootstrap

4c842be

Remove counter-productive automatic remote setup

8839eef

The remote over HTTPS needs constant authentication and user would need to remove and then re-add the remote

Add comment to keep non-interactive usage

25f0e43

Add Windows compatibility note

7c38ece

verbman merged commit 78815c0 into master Apr 24, 2024
7 checks passed

verbman deleted the training-upgrade branch April 24, 2024 05:06

This was referenced Apr 24, 2024

Fixed Spelling error #135

Closed

Permission denied when bootstrap script tries to rename the country-template directory under Windows 10 #82

Open

MattiSG added a commit that referenced this pull request Apr 24, 2024

Make errors in finding current version visible

fd46d00

Was silent in https://github.com/openfisca/country-template/actions/runs/8810580013, which led to failed merge in #139

MattiSG mentioned this pull request Apr 25, 2024

Update references to setup.py #142

Merged

MattiSG mentioned this pull request May 17, 2024

Publish to conda #124

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Training upgrade #139

Training upgrade #139

verbman commented Mar 20, 2024 •

edited

Loading

MattiSG left a comment

MattiSG commented Mar 28, 2024

MattiSG commented Mar 28, 2024

MattiSG left a comment

MattiSG Apr 13, 2024

verbman Apr 22, 2024

MattiSG Apr 13, 2024

verbman Apr 22, 2024

MattiSG Apr 13, 2024

verbman Apr 22, 2024

MattiSG left a comment

MattiSG left a comment

Training upgrade #139

Training upgrade #139

Conversation

verbman commented Mar 20, 2024 • edited Loading

MattiSG left a comment

Choose a reason for hiding this comment

MattiSG commented Mar 28, 2024

MattiSG commented Mar 28, 2024

MattiSG left a comment

Choose a reason for hiding this comment

MattiSG Apr 13, 2024

Choose a reason for hiding this comment

verbman Apr 22, 2024

Choose a reason for hiding this comment

MattiSG Apr 13, 2024

Choose a reason for hiding this comment

verbman Apr 22, 2024

Choose a reason for hiding this comment

MattiSG Apr 13, 2024

Choose a reason for hiding this comment

verbman Apr 22, 2024

Choose a reason for hiding this comment

MattiSG left a comment

Choose a reason for hiding this comment

MattiSG left a comment

Choose a reason for hiding this comment

verbman commented Mar 20, 2024 •

edited

Loading