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

Make the scripts compatible with py3 #1053

Merged
merged 2 commits into from
Aug 9, 2018

Conversation

magopian
Copy link
Contributor

@magopian magopian commented Aug 1, 2018

This is a very straightforward change involving mainly adding parens to the print statements.

The non-trivial changes are in openfisca_france/scripts/parameters/baremes_ipp/xls_to_yaml_raw.py and openfisca_france/scripts/parameters/baremes_ipp/yaml_raw_to_yaml_clean.py:
changing ur to r in regexes.

I'm really baffled that those tests would pass without failing in py3 on circleCI previously ...

@magopian magopian changed the title Make the code compatible with py3 [WIP] Make the code compatible with py3 Aug 1, 2018
@magopian
Copy link
Contributor Author

magopian commented Aug 1, 2018

Ok, the tests were passing because in circleCI it's only specifically running nosetests, while in the Makefile it's also compiling all the python files to check for syntax errors.

I also realized that the check-no-prints, check-syntax-errors and flake8 are not run at all in circleCI. I've opened # for that matter.

@magopian magopian force-pushed the compatibility-py3 branch 2 times, most recently from d85897c to 3d34c71 Compare August 1, 2018 11:42
@magopian magopian changed the title [WIP] Make the code compatible with py3 Make the code compatible with py3 Aug 1, 2018
@magopian
Copy link
Contributor Author

magopian commented Aug 1, 2018

This is now ready to review <3

@fpagnoux fpagnoux changed the title Make the code compatible with py3 Make the scripts compatible with py3 Aug 2, 2018
CHANGELOG.md Outdated

* Amélioration technique.
* Détails :
- Rends tous les scripts compatibles avec Python 3
Copy link
Member

Choose a reason for hiding this comment

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

J'édite le changelog, pour expliciter que le modèle était compatible avec Python3, mais pas certains scripts additionnels.

@fpagnoux
Copy link
Member

fpagnoux commented Aug 2, 2018

Ok, the tests were passing because in circleCI it's only specifically running nosetests, while in the Makefile it's also compiling all the python files to check for syntax errors.

I also realized that the check-no-prints, check-syntax-errors and flake8 are not run at all in circleCI. I've opened # for that matter.

The France model was already compatible with Python 3, but true, many additional scripts were not, thanks for dealing with that!

The makefile is a mess, and no one was brave enough to flake OpenFisca France after years of laxness.

@fpagnoux
Copy link
Member

fpagnoux commented Aug 2, 2018

Rebased, waiting for the tests to pass to merge

@vinyll
Copy link

vinyll commented Aug 7, 2018

Could you fix the conflicts @magopian?

@magopian
Copy link
Contributor Author

magopian commented Aug 9, 2018

@vinyll @fpagnoux rebased!

@fpagnoux fpagnoux merged commit 872053e into openfisca:master Aug 9, 2018
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