Solution to issue #18 in Python #26

Himanshu1495 · 2017-04-25T15:15:20Z

I have created the solution for issue #18 using Python 2.7 . It works perfectly when tested manually. If it looks good, then i request to merge this solution. Thanks.

msach22 · 2017-04-25T19:35:37Z

@Himanshu1495 good job, it looks like this method would work. Unfortunately, I don’t know any python or how to write python tests, do you have any good suggestions? Are you able to write some tests?

msach22 · 2017-04-26T04:44:24Z

@Himanshu1495 could you also write this as a function so we can pass in parameters?

Himanshu1495 · 2017-04-26T17:36:27Z

@msach22 , I have wrote the code as function so that you can pass parameters. I am working on creating tests for the same and will be done with it in 2 days.Should I push the code for test of this issue? Do you also need me to push a skeleton code for testing the python files in future too ?

msach22 · 2017-04-26T20:18:51Z

@Himanshu1495 Yes, please add the test to this issue :) And a skeleton for python testing would be fantastic!

I look forward to seeing your other commits :)

…s created.

Himanshu1495 · 2017-04-27T16:21:49Z

@msach22 , I have added tests for the code. Also I have created and pushed the skeleton code. Will you be able to review it ? I need approved review on my code to be able to merge the pull request.

Himanshu1495 · 2017-05-01T12:31:25Z

@msach22 , Have you checked the code ? Does it meet the requirements needed to contribute to this project ? Please let me know as soon as possible so that I may start working on other issues in this project.

Himanshu1495

@msach22 @songz I would like to get my code reviewed by you.

songz · 2017-05-04T17:06:25Z

@Himanshu1495 sorry for the delay, I'll review it today!

songz · 2017-05-04T17:07:25Z

solutions/18.py

+'''
+
+def missing_num(arr):
+	#get first element of array


don't put comments for obvious things. The goal should always to write your code in a way so that it doesn't require comments. For example, firstElement = arr[0]

@songz , Okay , i will modify that in a day.

songz · 2017-05-04T17:10:02Z

solutions/18.py

+	#get last element of array
+	end = arr[-1]
+	#create new array by iterating from start element to end element
+	new_arr = [x for x in range(start,end+1)]


What you are trying to do here is add up all the numbers from a to b. You don't need an array to do this (for loop? While loop? math formula?)

@songz , array has been used with a purpose of using a built-in function sum(name_of_array). Here is the line which uses sum() , return sum(new_arr)-sum(arr) . We can use a for loop to do the same task but in my opinion, sum() looks cleaner. If you insist on using a loop, I will certainly be able to do that too? Should I use loop or keep it as it is ?

@Himanshu1495 , try this, it looks much cleaner and doesn't involve creating a new array.

sum = 0; for x in range(start, end+1) sum += x;

@songz , Sure, I will do that .

Comments in the code have been removed. for loop has been used as discussed.

Tests have been modified as per the modified code of 18.py . The code has been tested and it passed the test cases.

Himanshu1495 · 2017-05-05T06:59:16Z

@songz , I have modified the code and its tests as per the discussion. Please check if it suffices the requirements of the solution.

songz · 2017-05-12T20:33:36Z

solutions/18.py

+	return new_sum - sum(arr)
+
+#example to run the code
+print missing_num([5,6,8,9])


please remove print statements

@songz , I have removed the print statements

songz · 2017-06-06T18:04:56Z

test/test18.py

@@ -0,0 +1,19 @@
+import unittest
+
+def missing_num(arr):


you should be importing from your solutions file.

@songz, can you please elaborate?

It doesn't make sense to write your solution in solutions file, then literally write the same code here. The point of tests is to test your code, so I would import the solutions file. I did a quick google search, perhaps this tutorial would help? https://www.codementor.io/sheena/python-path-virtualenv-import-for-beginners-du107r3o1

By the way, good job. If you pull this off we will officially support python :)

@songz Modified the test after almost an hour of searching for a way to do it. Phew! It was done finally. The solution will work on Linux perfectly. For Windows, a bit of modification might be required.
Thanks a lot for appreciating. It always feels good to know that my contribution is helping you :)

songz · 2017-06-06T18:05:20Z

solutions/pythontest_skeleton.py

@@ -0,0 +1,15 @@
+#Created by Himanshu Nachane (@Himanshu1495)


test should not be in solutions folder.

songz · 2017-06-13T16:50:08Z

test/pythontest_skeleton.py

@@ -0,0 +1,15 @@
+#Created by Himanshu Nachane (@Himanshu1495)


y do we need a pythontest_skeleton file?

@songz We need pythontest_skeleton so that new Python contributors can easily use it to create their tests. All they need to do is remove the comments in the file and replace it with actual data.

but y not just use your test file as a reference?

I would prefer to use your test file as reference rather than create a new skeleton

…ith new changes. Name of solution changed from 18.py to sol18.py to avoid conflict while importing the file for testing

Himanshu1495 · 2017-06-13T19:15:50Z

@songz Here is an explanation of what I did in the latest commit. import sys, this imports system parameters. sys.path.insert(0,'../solutions/'), this navigates to solutions folder and inserts its path at the start of the system parameters. from sol18 import missing_num, this imports the function missing_num from sol18.py. For an obvious reason, I needed to change the file name from 18.py to sol18.py because 18.py was needed to be imported as from 18 import missing_num and the 18 was interpreted as an integer which caused an error.

songz · 2017-06-13T20:01:56Z

test/pythontest_skeleton.py

@@ -0,0 +1,15 @@
+#Created by Himanshu Nachane (@Himanshu1495)


but y not just use your test file as a reference?

songz · 2017-06-13T20:02:42Z

test/test18.py

@@ -0,0 +1,14 @@
+import sys


rename to test/18.py. since file is already in the test folder, you don't need to put test in the file name

@songz renaming done :)

@songz pythontest_skeleton is deleted.

songz · 2017-06-13T21:57:01Z

test/pythontest_skeleton.py

@@ -0,0 +1,15 @@
+#Created by Himanshu Nachane (@Himanshu1495)


I would prefer to use your test file as reference rather than create a new skeleton

songz · 2017-06-15T00:03:54Z

solutions/sol18.py

@@ -0,0 +1,17 @@
+#Himanshu Nachane (@Himanshu1495)


can you rename this file to just 18.py please?

@songz, I tried keeping it 18.py but then I found out that Python files should not start with a number for the exact same reason. It would be good to keep the file name as sol18.py or anything which starts with a string.

Solution to issue #18 in Python

c77b47f

Modified the code,function created to allow passing the parameters

3068663

Tests for issue #18 added. SKELETON CODE to test python(2.7) solution…

01c3ab9

…s created.

Himanshu1495 commented May 3, 2017

View reviewed changes

songz reviewed May 4, 2017

View reviewed changes

Himanshu1495 added 2 commits May 5, 2017 12:24

Modification to 18.py

83580c3

Comments in the code have been removed. for loop has been used as discussed.

Modification to test18.py

80f3222

Tests have been modified as per the modified code of 18.py . The code has been tested and it passed the test cases.

Himanshu1495 added 4 commits May 9, 2017 21:13

Solution to issue #34 in Python 2.7

64650de

Merge branch 'master' of https://github.com/Himanshu1495/algorithms

fabec11

Delete 34.py

e2d1f35

Delete test34.py

ab97ccf

songz reviewed May 12, 2017

View reviewed changes

print statement removed from solution

7a20b9b

songz reviewed Jun 6, 2017

View reviewed changes

Test skeleton moved from solutions to tests

a0d7b6e

songz reviewed Jun 13, 2017

View reviewed changes

Test modified to eliminate redundancy. pythontest_skeleton modified w…

2b03a34

…ith new changes. Name of solution changed from 18.py to sol18.py to avoid conflict while importing the file for testing

songz reviewed Jun 13, 2017

View reviewed changes

songz requested changes Jun 13, 2017

View reviewed changes

Renamed test18.py to 18.py and deleted pythontest_skeleton

0a2acc5

songz reviewed Jun 15, 2017

View reviewed changes

songz approved these changes Jul 7, 2017

View reviewed changes

msach22 merged commit 100b5c1 into llipio:master Jul 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solution to issue #18 in Python #26

Solution to issue #18 in Python #26

Himanshu1495 commented Apr 25, 2017

msach22 commented Apr 25, 2017

msach22 commented Apr 26, 2017

Himanshu1495 commented Apr 26, 2017

msach22 commented Apr 26, 2017

Himanshu1495 commented Apr 27, 2017

Himanshu1495 commented May 1, 2017

Himanshu1495 left a comment

songz commented May 4, 2017

songz May 4, 2017

Himanshu1495 May 4, 2017

songz May 4, 2017

Himanshu1495 May 4, 2017 •

edited

Loading

songz May 4, 2017

Himanshu1495 May 5, 2017

Himanshu1495 commented May 5, 2017 •

edited

Loading

songz May 12, 2017

Himanshu1495 May 13, 2017

songz Jun 6, 2017

Himanshu1495 Jun 6, 2017

songz Jun 13, 2017

Himanshu1495 Jun 13, 2017 •

edited

Loading

songz Jun 6, 2017

songz Jun 13, 2017

Himanshu1495 Jun 13, 2017 •

edited

Loading

songz Jun 13, 2017

songz Jun 13, 2017

Himanshu1495 commented Jun 13, 2017 •

edited

Loading

songz Jun 13, 2017

songz Jun 13, 2017

Himanshu1495 Jun 14, 2017

Himanshu1495 Jun 14, 2017

songz Jun 13, 2017

songz Jun 15, 2017

Himanshu1495 Jun 15, 2017

		@@ -0,0 +1,15 @@
		#Created by Himanshu Nachane (@Himanshu1495)

Solution to issue #18 in Python #26

Solution to issue #18 in Python #26

Conversation

Himanshu1495 commented Apr 25, 2017

msach22 commented Apr 25, 2017

msach22 commented Apr 26, 2017

Himanshu1495 commented Apr 26, 2017

msach22 commented Apr 26, 2017

Himanshu1495 commented Apr 27, 2017

Himanshu1495 commented May 1, 2017

Himanshu1495 left a comment

Choose a reason for hiding this comment

songz commented May 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Himanshu1495 May 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Himanshu1495 commented May 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Himanshu1495 Jun 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Himanshu1495 Jun 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Himanshu1495 commented Jun 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Himanshu1495 May 4, 2017 •

edited

Loading

Himanshu1495 commented May 5, 2017 •

edited

Loading

Himanshu1495 Jun 13, 2017 •

edited

Loading

Himanshu1495 Jun 13, 2017 •

edited

Loading

Himanshu1495 commented Jun 13, 2017 •

edited

Loading