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

Automatically import some numpy functions in formulas #632

Merged
merged 3 commits into from
Mar 24, 2017

Conversation

fpagnoux
Copy link
Member

@fpagnoux fpagnoux commented Dec 7, 2016

Automatically import some numpy functions in formulas, as they are documented standard openfisca helpers:

  • max_
  • min_
  • not_
  • where
  • select

@MattiSG MattiSG requested a review from cbenz March 2, 2017 15:30
@MattiSG
Copy link
Member

MattiSG commented Mar 2, 2017

Nécessite un rebase, un ajout dans le CHANGELOG et un bump de version.

Le 👍 de @benjello me fait penser que cet ajout serait utile 😉
Mais peut-être ces helpers devraient-ils être automatiquement fournis par core… ?

@fpagnoux pourquoi cette PR n'a-t-elle pas été intégrée ?

Copy link
Member

@cbenz cbenz left a comment

Choose a reason for hiding this comment

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

Importing everything from a package is a bad practice.

I know it's convenient but it makes completely impossible to track the origin of a function or a variable.
Code analysis tools like flake8 are stuck too.

Given the import * were already used before this PR I don't request changes.

@MattiSG
Copy link
Member

MattiSG commented Mar 20, 2017

@fpagnoux was this superseded by openfisca/openfisca-core#467? 🤔 If yes, please close this PR.

@fpagnoux fpagnoux force-pushed the put-some-numpy-functions-in-base branch from 6f67d2e to 696920b Compare March 23, 2017 15:59
@fpagnoux fpagnoux requested a review from cbenz March 23, 2017 18:12
Copy link
Member

@cbenz cbenz left a comment

Choose a reason for hiding this comment

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

Good!

@fpagnoux fpagnoux force-pushed the put-some-numpy-functions-in-base branch from c890415 to 95c2490 Compare March 24, 2017 13:08
@fpagnoux fpagnoux merged commit 5f9a3cf into master Mar 24, 2017
@fpagnoux fpagnoux deleted the put-some-numpy-functions-in-base branch March 24, 2017 14:04
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