[READY]Add coverage in CI #8

utensil · 2018-09-21T14:54:34Z

This PR:

Move tests to a seperate folder test
adds coverage except it's using pytest and CodeCov instead of nosetests and coveralls in pygae/clifford
adds TravisCI and CodeCov badges:
Remove NumPy dependency
Add simple install and test instructions in README
Validate existing Jupyter notebooks using nbval

hugohadfield

Looks good 👍

utensil · 2018-09-22T02:57:07Z

@hugohadfield Sorry, after I requested the review last night, without seeing your approval, I felt there are more to be done in a PR this morning (in my timezone), so I added a few commits.....

Especially please see my changes in README and see if it's appropriate.

codecov · 2018-09-22T05:56:04Z

Codecov Report

❗ No coverage uploaded for pull request base (master@7c664a1). Click here to learn what that means.
The diff coverage is 22.22%.

@@           Coverage Diff           @@
##             master     #8   +/-   ##
=======================================
  Coverage          ?    40%           
=======================================
  Files             ?      7           
  Lines             ?   4959           
  Branches          ?      0           
=======================================
  Hits              ?   1984           
  Misses            ?   2975           
  Partials          ?      0

Impacted Files	Coverage Δ
galgebra/lt.py	`11.07% <12.5%> (ø)`
galgebra/printer.py	`30.3% <16.66%> (ø)`
galgebra/mv.py	`37.07% <50%> (ø)`
galgebra/ga.py	`62.75% <50%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c664a1...32a5334. Read the comment docs.

utensil · 2018-09-22T06:35:17Z

I've changed from Coveralls to CodeCov ( I've used both in the past ) because the latter feels more modern and Coveralls seems to be quite slow.

Also I've chosen pytest over nosetests because I was using it and I saw the following on https://nose.readthedocs.io/en/latest/:

Nose has been in maintenance mode for the past several years and will likely cease without a new person/team to take over maintainership. New projects should consider using Nose2, py.test, or just plain unittest/unittest2.

and when I go to https://github.com/nose-devs/nose2 , I saw:

Even though unittest2 plugins never arrived, nose2 is still being maintained! ...... However, given the current climate, with much more interest accruing around pytest, nose2 is prioritizing bugfixes and maintenance ahead of new feature development.

utensil · 2018-09-22T11:07:52Z

Oops, I found a bug in my last PR when porting to 3, trying to fix it and add tests for it now.

hugohadfield

Skimming this, looks good! Maybe clear the output of the jupyter notebooks included?

arsenovic · 2018-09-24T12:14:12Z

@hugohadfield we should use nbval with clifford docs. maybe just its lax option, since we strip the output.
ill take a look

hugohadfield · 2018-09-24T13:24:59Z

Ok cool, I actually have very little idea how these notebook style tests work etc, I'll have a read into them as well when I get a chance!

utensil · 2018-09-24T14:09:51Z

@hugohadfield we should use nbval with clifford docs. maybe just its lax option, since we strip the output.
ill take a look

@arsenovic nbval checks both LaTeX output and text output (which is not visible in jupyter notebook), I've verified them with many local runs before I fix the notebooks and corresponding codes in galgebra itself. It's very helpful if we have many notebooks and want to make sure they are not outdated. That's also why I chose to keep the output for now.

utensil · 2018-09-24T14:11:15Z

examples/ipython/second_derivative.ipynb

    "from sympy import symbols\n",
-    "from printer import Format\n",
+    "from galgebra.printer import Format\n",
    "\n",


The way to import is changed now.

hugohadfield · 2018-09-24T14:12:00Z

Sounds sensible @utensil :)

utensil · 2018-09-24T14:13:05Z

galgebra/lt.py

    dict_rep = {}
    n = len(basis)
    if mat_rep.rows != n or mat_rep.cols != n:
-        raise ValueError('Matrix and Basis dimensions not equal for Matrix = ' + str(mat))
+        raise ValueError('Matrix and Basis dimensions not equal for Matrix = ' + str(mat_rep))


This was an undefined variable, caused by a typo.

utensil · 2018-09-24T14:16:34Z

galgebra/lt.py

    n_range = list(range(n))
    for row in n_range:
        dict_rep[basis[row]] = S(0)
        for col in n_range:
-            dict_rep[basis[row]] += mat_rep[row,col]*basis[col]
+            dict_rep[basis[row]] += mat_rep[col,row]*basis[col]


The original usage of the class Transpose which calls _entry with a parameter expand, but it will call the _entry method of normal matrix class which don't have such a parameter. So I removed the usage of Transpose, instead directly change the order of indexes to get the result.

utensil · 2018-09-24T14:19:30Z

galgebra/mv.py

@@ -1048,7 +1048,7 @@ def proj(self, bases_lst):
        return Mv(obj, ga=self.Ga)

    def dual(self):
-        mode = ga.Ga.dual_mode_value
+        mode = self.Ga.dual_mode_value


This was a reference to ga. It won't work after the removal of circular dependency. And given the context, it actually meant self.Ga, just like in other methods. It's a hidden typo which isn't easy to be found before the removal of circular dependency on ga.

utensil · 2018-09-24T14:20:02Z

galgebra/mv.py

+        # self.terms.sort(key=operator.itemgetter(1), cmp=Pdop.compare)
+        # terms are in the form of (coef, pdiff)
+        # so we need to first extract pdiff and then use Pdop.compare to compare
+        self.terms.sort(key=cmp_to_key(lambda term1, term2 : Pdop.compare(term1[1], term2[1])))
        return


This is where I was wrong in last PR.

utensil · 2018-09-24T14:22:06Z

galgebra/lt.py

@@ -328,6 +327,9 @@ def __rmul__(self, LT):
        else:
            raise TypeError('Cannot have LT as left argument in Lt __rmul__\n')

+    def __repr__(self):
+        return str(self)
+


Without this, it will product the correct LaTeX output with an incorrect text output invisible in a Jupyter notebook. Found it with nbval.

utensil · 2018-09-24T14:25:28Z

Thanks for the review, now I'll merge into master. (also added a few comments on some code which fixed problems but not quite visible in the review). Further fix could be done in a separate PR.

utensil force-pushed the coverage branch 3 times, most recently from f5fb51e to 97beb1a Compare September 21, 2018 15:02

Add coveralls

db25b9d

utensil force-pushed the coverage branch from 97beb1a to db25b9d Compare September 21, 2018 15:06

Add badges

0023313

utensil self-assigned this Sep 21, 2018

utensil mentioned this pull request Sep 21, 2018

A roadmap towards improving tests and CI #3

Closed

7 tasks

utensil requested review from arsenovic and hugohadfield September 21, 2018 15:26

hugohadfield approved these changes Sep 21, 2018

View reviewed changes

utensil added 7 commits September 22, 2018 10:04

Move tests to a seperate folder test

ac8215b

Dead code clean up

ef25aaa

Add comments for test commands

a07dcee

Make test a module

6793b1c

Make python setup.py test work

b08d2a1

Remove NumPy dependency

73db2fa

Add install and test instructions in README

04bca73

Change coveralls to codecov

09e29ec

utensil changed the title ~~Add coveralls~~ Add coverage in CI Sep 22, 2018

Change to CodeCov badge

9f75fdc

Fix _print_Pow

d81364b

Fix sort_terms (first key then cmp)

e727e9e

utensil force-pushed the coverage branch from f46e26a to e727e9e Compare September 22, 2018 11:50

utensil added 2 commits September 22, 2018 19:54

Validate Jupyter notebook dop.ipynb

e78593a

Validated all nbs in examples/ipython/

03b3713

Fix some PyLint warnings

c119b5b

utensil force-pushed the coverage branch from 66594c4 to 153114c Compare September 22, 2018 14:38

Fix lt.py and validate with doc/ipython/simple_ga_test.ipynb

32a5334

utensil force-pushed the coverage branch from 153114c to 32a5334 Compare September 23, 2018 04:42

utensil changed the title ~~Add coverage in CI~~ [READY]Add coverage in CI Sep 23, 2018

utensil requested a review from hugohadfield September 23, 2018 07:15

hugohadfield reviewed Sep 24, 2018

View reviewed changes

hugohadfield approved these changes Sep 24, 2018

View reviewed changes

utensil commented Sep 24, 2018

View reviewed changes

utensil merged commit 21ed56d into master Sep 24, 2018

utensil mentioned this pull request Feb 14, 2019

0.4.3 release #11

Merged

5 tasks

utensil deleted the coverage branch February 25, 2019 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[READY]Add coverage in CI #8

[READY]Add coverage in CI #8

utensil commented Sep 21, 2018 •

edited

hugohadfield left a comment

utensil commented Sep 22, 2018 •

edited

codecov bot commented Sep 22, 2018 •

edited

utensil commented Sep 22, 2018 •

edited

utensil commented Sep 22, 2018

hugohadfield left a comment

arsenovic commented Sep 24, 2018

hugohadfield commented Sep 24, 2018

utensil commented Sep 24, 2018

utensil Sep 24, 2018

hugohadfield commented Sep 24, 2018

utensil Sep 24, 2018

utensil Sep 24, 2018

utensil Sep 24, 2018

utensil Sep 24, 2018

utensil Sep 24, 2018

utensil commented Sep 24, 2018

[READY]Add coverage in CI #8

[READY]Add coverage in CI #8

Conversation

utensil commented Sep 21, 2018 • edited

hugohadfield left a comment

Choose a reason for hiding this comment

utensil commented Sep 22, 2018 • edited

codecov bot commented Sep 22, 2018 • edited

Codecov Report

utensil commented Sep 22, 2018 • edited

utensil commented Sep 22, 2018

hugohadfield left a comment

Choose a reason for hiding this comment

arsenovic commented Sep 24, 2018

hugohadfield commented Sep 24, 2018

utensil commented Sep 24, 2018

utensil Sep 24, 2018

Choose a reason for hiding this comment

hugohadfield commented Sep 24, 2018

utensil Sep 24, 2018

Choose a reason for hiding this comment

utensil Sep 24, 2018

Choose a reason for hiding this comment

utensil Sep 24, 2018

Choose a reason for hiding this comment

utensil Sep 24, 2018

Choose a reason for hiding this comment

utensil Sep 24, 2018

Choose a reason for hiding this comment

utensil commented Sep 24, 2018

utensil commented Sep 21, 2018 •

edited

utensil commented Sep 22, 2018 •

edited

codecov bot commented Sep 22, 2018 •

edited

utensil commented Sep 22, 2018 •

edited