Skip to content

Use declarative config for setup.py and enable coverage testing#422

Merged
fredrik-johansson merged 11 commits into
mpmath:masterfrom
skirpichev:setup.cfg-and-cov
Feb 3, 2019
Merged

Use declarative config for setup.py and enable coverage testing#422
fredrik-johansson merged 11 commits into
mpmath:masterfrom
skirpichev:setup.cfg-and-cov

Conversation

@skirpichev

@skirpichev skirpichev commented Dec 23, 2018

Copy link
Copy Markdown
Collaborator

Major changes:

  • use setuptools and add a declarative config for setup.py
  • turn on coverage testing (see Use codecov.io for code coverage reports in Travis #323). You can enable mpmath repo on codecov.io to see coverage statistics (I think, it will be available only after PR merge).
  • use setuptools_scm to track versions (as you can see, release procedure could be reduced just to commit tagging). Unfortunately, this require dropping py3.3 support.

Minor stuff:

  • use xenial dist on Travis
  • pep8 -> pycodestyle renaming
  • adapt LICENSE, so github will recognize it
  • add more classifiers for pypi.org
  • unbundled the six library

I can split this PR if you prefer.

see
https://setuptools.readthedocs.io/en/latest/setuptools.html#configuring-setup-using-setup-cfg-files

Also this commit expands some metadata, e.g. classifiers and
project_urls.  Testing requirements are specified with "tests" extras.
Other development requirements are tracked with "develop" extras.

Peferences to extras are supported in tests_require since setuptools
v36.7.0, hence additional entry in setup_requires.
@skirpichev skirpichev changed the title Use declarative config for setup.py and enable coverage testing [wip] Use declarative config for setup.py and enable coverage testing Dec 23, 2018
We run coverage tests only in py3.6
Also drop testing on py3.3
In this way Github recognize the LICENSE as BSD
@skirpichev skirpichev changed the title [wip] Use declarative config for setup.py and enable coverage testing Use declarative config for setup.py and enable coverage testing Dec 23, 2018
@fredrik-johansson

Copy link
Copy Markdown
Collaborator

Thanks, and apologies for the long delay!

use setuptools and add a declarative config for setup.py

Could you explain the advantages (and possible disadvantages) of this?

Could you roll back the unbundling of six.py? I prefer to avoid the external dependency.

@skirpichev

Copy link
Copy Markdown
Collaborator Author

Could you explain the advantages (and possible disadvantages) of this?

More declarative config, no code at all. And all projects settings are in one place.

Not sure about disadvantages, maybe version requirements for the setuptools, that disallow ancient stuff. I can lower this down to 30.3.0. The price is some repetition for *requires options: we can't reference extras requirements like

tests_require = mpmath[tests]

Could you roll back the unbundling of six.py? I prefer to avoid the external dependency.

Can you explain some technical reasons behind of this? pip can handle external dependencies well, by design. In turn, you already have very outdated six.py file, almost uncovered by tests. An option may be copy&paste only very tiny part of the six, which is actually used and will be well maintained.

@fredrik-johansson

Copy link
Copy Markdown
Collaborator

It looks like we only depend on exec_ and print_ from six.py. I suppose we could just move those to backend.py? We don't even need the full print_, really; it's just used in a handful of places for concatenating strings.

No dependency = one less thing that can go wrong.

@skirpichev

Copy link
Copy Markdown
Collaborator Author

Ok, dependency on six was removed. Note bad coverage on this new diff - implemented exec_() on PY2 wasn't covered at all. Should we run coverage tests on PY2 just for this tiny function? (That is an example, why dependencies are good for business - other people will test their stuff.)

(BTW, codecov.yml taken from the diofant, you may want less strict patch requirements.)

@fredrik-johansson

Copy link
Copy Markdown
Collaborator

Ok, dependency on six was removed. Note bad coverage on this new diff - implemented exec_() on PY2 wasn't covered at all.

That must be because PY2 isn't covered at all. exec_ is heavily used, to implement the binary operations for the mpf type and for hypergeometric series!

@skirpichev

skirpichev commented Feb 3, 2019

Copy link
Copy Markdown
Collaborator Author

That must be because PY2 isn't covered at all.

Of course. The question was: should we enable coverage testing on PY2 just for that single function?

@fredrik-johansson

Copy link
Copy Markdown
Collaborator

I don't understand the problem.

@skirpichev

Copy link
Copy Markdown
Collaborator Author

I don't understand the problem.

For example, travis build will take longer time (COVERAGE=on jobs more then 2x times slower then usual).

Ok, now code coverage collected for 2.7 too. Please restart failed job.

@fredrik-johansson fredrik-johansson merged commit 8613130 into mpmath:master Feb 3, 2019
@fredrik-johansson

Copy link
Copy Markdown
Collaborator

Good! And now the fun game of raising the coverage% begins :)

@skirpichev skirpichev deleted the setup.cfg-and-cov branch February 3, 2019 13:37
@fredrik-johansson

Copy link
Copy Markdown
Collaborator

Does coverage testing ignore doctests?

@skirpichev

Copy link
Copy Markdown
Collaborator Author

No. pytest is collecting doctests per default (i.e. we run it with --doctest-modules):

$ py.test mpmath/calculus/optimization.py
============================= test session starts ==============================
platform linux -- Python 3.7.1, pytest-4.2.0, py-1.7.0, pluggy-0.8.1
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home/sk/src/mpmath/.hypothesis/examples')
rootdir: /home/sk/src/mpmath, inifile: setup.cfg
plugins: xdist-1.26.1, timeout-1.3.3, forked-1.0.1, cov-2.6.1, hypothesis-4.4.3
collected 3 items                                                              

mpmath/calculus/optimization.py ...                                      [100%]

=========================== 3 passed in 0.37 seconds ===========================

But there may be problems, which I miss, may be with dynamic docstrings or something.

@fredrik-johansson

Copy link
Copy Markdown
Collaborator

Example: libmp.libintmath.stirling1 is reported as not covered, but should be covered via the doctest for mpmath.stirling1.

Another example: functions.qfunctions.qhyper is reported as not covered, but has its own doctests.

@skirpichev

Copy link
Copy Markdown
Collaborator Author

libmp.libintmath.stirling1 is reported as not covered, but should be covered via the doctest for mpmath.stirling1.
Another..

I see. These seems to be the mentioned above cases with dynamic __doc__.

@fredrik-johansson

Copy link
Copy Markdown
Collaborator

That's bizarre -- the docstrings in functions/qfunctions.py are not dynamic.

In either case, if the test runner doesn't run all doctests, that's not good.

@skirpichev

Copy link
Copy Markdown
Collaborator Author

the docstrings in functions/qfunctions.py are not dynamic

Hmm, indeed. This case seems to be related with broken defun* decorators, see #432.

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.

2 participants