Skip to content

Conversation

guillaume-chevalier
Copy link
Contributor

Bonjour,

I added the convolve iteration algorithm with tests: a simple way to do convolutions on lists.

  • If creating a new file :

    • added links to it in the README files ?
    • included tests with it ?
    • added description (overview of algorithm, time and space compleixty, and possible edge case) in docstrings ?
  • if done some changes :

    • wrote short description in the PR explaining what the changes do ?
    • Fixes #[issue number] if related to any issue
  • other

@guillaume-chevalier
Copy link
Contributor Author

Note: the test test_plus_one_v3 fails and was already failing before opening this PR, everything else works:

py36 runtests: commands[0] | python3 -m unittest discover tests
.................E...............................................................................................................................................................................................................................
======================================================================
ERROR: test_plus_one_v3 (test_array.TestPlusOne)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/keon/algorithms/tests/test_array.py", line 252, in test_plus_one_v3
    self.assertListEqual(plus_one_v3([0]), [1])
  File "/home/travis/build/keon/algorithms/algorithms/arrays/plus_one.py", line 45, in plus_one_v3
    num_arr[idx] = (num_arr[idx] + 1) % 10
TypeError: list indices must be integers, not tuple
----------------------------------------------------------------------
Ran 241 tests in 0.433s
FAILED (errors=1)
ERROR: InvocationError for command '/home/travis/build/keon/algorithms/.tox/py36/bin/python3 -m unittest discover tests' (exited with code 1)
___________________________________ summary ____________________________________
ERROR:   py36: commands failed
The command "tox -e $TOX_ENV" exited with 1.
0.45s$ pip3 uninstall -y algorithms
Uninstalling algorithms-0.1.0:
  Successfully uninstalled algorithms-0.1.0

Copy link
Owner

@keon keon left a comment

Choose a reason for hiding this comment

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

thanks! I left some comments

@guillaume-chevalier
Copy link
Contributor Author

Thanks for the review and for your time!

@guillaume-chevalier
Copy link
Contributor Author

I made the changes.

@guillaume-chevalier
Copy link
Contributor Author

@keon I finally decided to make the requested changes.

@keon keon merged commit 006cefd into keon:master Mar 26, 2019
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