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

[Project Euler] Algorithms do not follow coding styles #2786

Open
kalpanajangra opened this issue Oct 5, 2020 · 5 comments
Open

[Project Euler] Algorithms do not follow coding styles #2786

kalpanajangra opened this issue Oct 5, 2020 · 5 comments

Comments

@kalpanajangra
Copy link
Contributor

@kalpanajangra kalpanajangra commented Oct 5, 2020

Many algorithms in Project Euler do not follow coding styles as mentioned by @dhruvmanila here, or even the standard coding guidelines.

Notes:

  • A lot of inconsistencies with the solution functions (a lot of them aren't named as "solution")
  • docstring inconsistencies:
>```docstring```
vs
>```
docstring```
The following are considered to be bad and may be requested to be improved:
>>> x = x + 2	# increased by 2
This is too trivial. Comments are expected to be explanatory. For comments, you can write them above, on or below a line of code, as long as you are consistent within the same piece of code.

But there are places where this isn't practised.

print(count) # --> 443839

The list isn't complete.
As a beginner who's looking to either understand/contribute, this makes it a bit difficult.

@dhruvmanila

Loading…

Member

@dhruvmanila dhruvmanila commented Oct 5, 2020

UPDATE: As this is getting closer and closer to being fixed and the number of files per directory is reducing, you can create a PR fixing files in maximum 2 directories.

This is a completely valid issue. The thing is:

  • Those initial solutions were submitted a long time ago
  • I don't think consistency was maintained back then

Now as the number of solutions is increasing we need to enforce some of the consistency. One of the main ones was as I mentioned in #2695 (comment) but as you said there are many more. We can start with this and fix one issue at a time.

Due to some of the solutions not containing solution() function or requires some number of positional arguments we have decided to skip this script for now which checks for the solutions for every problem present. We can start fixing this first and then move on to other issues mentioned.

Submit the fix for one maximum two directories at a time and once that is approved, submit another one.

These are the errors which need to be fixed before we can start running the script in our tests:
https://travis-ci.com/github/TheAlgorithms/Python/jobs/394777235#L240-L312

These two are the main messages to look for in the above log:

  1. solution() missing 1 required positional argument: 'n'
  2. module 'sol1.py' has no attribute 'solution'
  • The first one says that we have a solution() function but that requires a positional argument so to fix that put the default value for those parameters as the question input such that when we call the function without any arguments it returns the answer.
  • The second one says that the specific file doesn't contain any solution() function so we need to either add the function or change the name of the present function, which returns the answer, to solution() and if it takes any positional arguments, to do the same thing as mentioned in the first point.

Current Status:

  • problem_234/sol1.py: solution() missing 1 required positional argument: 'n'
kalpanajangra added a commit to kalpanajangra/Python that referenced this issue Oct 5, 2020
bobluppes added a commit to bobluppes/Python that referenced this issue Oct 5, 2020
@CMeza99 CMeza99 mentioned this issue Oct 6, 2020
10 of 14 tasks complete
@CMeza99 CMeza99 mentioned this issue Oct 6, 2020
10 of 14 tasks complete
@vovapi vovapi mentioned this issue Oct 6, 2020
11 of 14 tasks complete
dhruvmanila added a commit that referenced this issue Oct 7, 2020
* fix code style in problem 76

Signed-off-by: joan.rosellr <joan.rosellr@gmail.com>

* Update sol1.py

* Update sol1.py

* Remove trailing whitespace

Co-authored-by: Dhruv <dhruvmanila@gmail.com>
@M-Smits

Loading…

@M-Smits M-Smits commented Oct 7, 2020

@dhruvmanila Is the task list up to date?
Because I see a lot of commits and I am not sure where I can still help.

@pyaillet pyaillet mentioned this issue Oct 7, 2020
11 of 12 tasks complete
Hyftar referenced this issue Oct 8, 2020
* name method solution in project_euler/problem99

* rename function
@dhruvmanila

Loading…

Member

@dhruvmanila dhruvmanila commented Oct 8, 2020

@M-Smits Yes, but keep in mind that the task list only removes a fix if it's merged. It might be the case that the fix is present in the list but someone already opened a PR, you have to confirm that yourself.

@UsmanAhmadSaeed

Loading…

@UsmanAhmadSaeed UsmanAhmadSaeed commented Oct 8, 2020

@dhruvmanila I'm a beginner in Python and would love to contribute. Could you please guide me with any of the issues and I'll take it from there?
Thanks

@manthan2501

Loading…

@manthan2501 manthan2501 commented Oct 9, 2020

I would like to work on this in python. kindly assign me this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.