23
I'd have to rate your program a "D-Minus".
incorrectly labels 1, 0, and all negative numbers as prime!
runs in \$O(n)\$ time, instead of \$O(\sqrt n)\$ time
runs in \$O(n)\$ space, instead of \$O(1)\$ space
Other deficiencies:
'''docstrings''' should be used for describing "how" to use the module/class/method; comments should be used to explain how the ...
15
Apart from what's already mentioned I would like to point out that
the file super.picture.jpg will be converted to super.png
That can be a problem if someone runs your program in a loop and iterates through a folder with files named anniversary.1.jpg anniversary.2.jpg....
Instead, because you have used endswith('.jpg') you can just use a substring of ...
13
First, thank you for including a docstring that explained how the script works; it saved me a good couple of minutes in puzzling it out. :) It turned out to be a lot simpler than I initially thought it was, which suggests an easy path to optimizing it for space.
The error that your program hits with large numbers is a MemoryError, which is a result of the ...
12
Since the goal is the best possible implementation of this algorithm, I'd suggest the following. However, faster algorithms do exist.
To conform to PEP8 make sure you have two blank lines after your imports and surrounding function definitions.
Since, you aren't editing each item, but rather adding and removing items until the list is sorted, I'd use the ...
11
To reduce memory usage you can exploit the mechanics of zip and iter / iterator / generator expression.
Make tmp an iterator.
You can achieve this by changing the brackets from [] to (); changing it from a list comprehension to a generator expression.
You can alternately wrap the list comprehension in an iter call. However that would still be using \$O(n)\...
10
Code showing some of the points AJNeufeld mentioned.
Further points:
You can check most false states in one condition
Because of this, you only need to check factors from a minimum of 3
Working through the possible factors in ascending order will produce a quicker result. E.g. 1200 is not divisible by 1199, but is by 3.
You only need to check factors up ...
10
You should read PEP 8 and get a linter; pycodestyle, pylint, flake8, prospector, coala. It doesn't matter which one, just get one and use it.
Indenting with 3 spaces is awful, no-one can easily interact with your code.
I've never in my life seen 3 spaces for indentation, it makes me think your post is a joke.
Variables like s, l and ii are useless.
It's ...
10
Consider using __main__ to make your script easier to import (see here ).
Also, consider the use of functions to isolate the different steps, so that it is easier to add new functionalities in the future.
One last thing, you could use a more explicit log message, something along the lines of Converting {filename} from jpg to png instead of Lets go, maybe ...
9
Your code looks well-formatted, it's easy to read and to follow, and the explanation you gave matches the code exactly. Well done. :)
for item in my_list:
This statement looks strange since in the body of this for loop, you neither use item nor my_list. You can express the idea of that code more directly:
for _ in range(len(my_list)):
The variable _ is ...
9
Your usage of iter() and next() is unnecessarily confusing. This comes from the requirement of using a set as an input, where sets are not indexable.
If you passed in a list, then instead of this:
elif l == 1: return next(iter(s))
you could write:
elif l == 1: return s[0]
which is much clearer. It gets even clearer when you remove the iterator ...
8
I believe you could reduce it to \$O(n\log{n}+m\log{n}+n m)\$ if you want.
Sort the inputs, then iterate over the tests and for each one, do a two variable iteration over the inputs, where you start as close to the test value/2, and move one index in the increasing direction and the other in the decreasing direction depending on whether the sum is less ...
6
Your function code can be simplified to one line.
from itertools import combinations
from typing import List
def array_sum(inputs: List[int], tests: List[int]) -> bool:
"""
Determines if any two integers in `inputs`
add up to any integer in `tests`.
:param List[int] inputs: Input data
:param List[int] tests: Numbers to test against ...
6
Quick bits
You have some issues that some linters would pick up:
I would suggest moving your main code into a function. So that it doesn't pollute the global namespace.
You've got some trailing whitespace.
Add some docstrings to your code. Even something basic like "Fetch words in answers."
Your imports are kinda all over the place. I can't make any sense ...
5
pr means the number we'll be working on
dl is a dictionary that contains our the dividend and the reaminder
x is the dividend
e is a list that contains all remainders
These variable names are very short and don't have a lot of meaning.
You can safe a lot of comments by naming your variables: number_to_check, remainders, dividend, remainder_list
Not ...
5
Some high level coding style notes:
Try to eliminate code that looks like it's copied and pasted (DRY - Don't Repeat Yourself), and define it in functions. Your various color functions are mostly identical with the exception of one digit (the color code) -- you can avoid repeating yourself by defining a function that implements the shared part and defining ...
5
function
I would abstract this into a function you can call. Then you can also easily incorporate a few checks to see whether the source path really is a directory, that the destination path actually exists,...
pathlib.Path
Has a lot of convenience methods that help with file path handling.
from PIL import Image
from pathlib import Path
def ...
5
So, you want to count something and afterwards get the top k? That sounds like a job for collections.Counter!
from collections import Counter
def leading_candidates(votes, timestamp, k):
vote_counts = Counter(vote['candidate']
for vote in votes
if vote['timestamp'] <= timestamp)
return [...
4
Instead of using nested for loops for your case - Python provides a more flexible and performant features with itertools module, suitably - itertools.combinations(iterable, r) which returns generator of r length subsequences of elements from the input iterable.
Besides, don't forget about good naming: arraySum is not a good one (at least array_sum). I'd ...
4
There are a couple of issues with destructors, firstly is the issue with closing pipes on Linux/Unix as discussed here (though the pipe is actually no longer necessary once we fix the second issue). Secondly the functools.partial method appears to capture a reference to self which causes the wrapper object to not be destructed when expected, I have fixed ...
4
In general the code is simple and concise.
Depending on the target usage, apart from what other answers suggest, I would add:
support for files ending on ". jpeg" not only .jpg;
likewise I would make filename filter also find uppercase or mixed case JPEG files;
if I had to deal with unreliable sources I would add also Image.verify() and/or imghdr checks to ...
3
I strongly feel that you should separate the logic of the graphics and the algorithm of solving the sudoku. The algorithm is just some operation defined on some data structure of your choosing (actually a good way to regard solving sudokus is as extending a colouring of a graph).
I would write this algorithm to act on whatever data structure is most ...
3
In addition, I'd recommend using a for loop instead of a while loop here just so that it's more clear what's going on. And I'd put the entire for loop in a function so you could do something like
if isPrime(pr):
print(pr, " is a prime number.")
else:
print(pr, " is not a prime number.")
This will also make it so you can use return() instead of exit(...
3
Your problem is at least as hard as 3SUM, https://en.wikipedia.org/wiki/3SUM (in fact it is very likely equivalent with a little thought).
Let us first see how to reduce a 3SUM problem to your problem.
3SUM says I have an array \$X\$, and I want to find out if there are \$x\$, \$y\$, \$z\$ in \$X\$ such that $$x + y + z = 0$$
Let us define \$A = X\$ and \$...
3
class Node(object):
def __init__(self, item, left=None, right=None):
self.item = item
self.left = None
self.right = None
The parameters left and right are not used in the constructor! Remove them, or actually use them.
def _add(self, value):
new_node = Node(value)
if not self.item:
self.item = ...
3
well done providing doc strings
sticking to the Style Guide for Python Code makes Python code easier to grasp, especially for someone who didn't write it
to make the naming more convincing, you should factor out sequential_deterministic_stand_in_for_seeing_the_required_result() and sequential_deterministic_stand_in_for_checking_the_result()
As a bonus, this ...
3
Your code can probably utilize OOP.
However, the way you're currently using objects is not what I'd call good.
But before we start adding in complicated objects, it looks like we can do everything you want with a couple of functions:
Read the title of the document:
self.title = file.readline()
You read until you get to NNODES:
while not file....
3
You don't need to actually build a whole list of the digits. It's unnecessary memory consumption. You can pass a generator to max.
use python's -= syntax when subtracting from a variable.
The break is unnecessary as it's covered by the while condition
Final code:
n=int(input())
count=0
while n:
n -= max(int(i) for i in str(n))
count += 1
...
3
A couple of things that I would suggest that you do:
A level of nesting can be removed by using a generator expression to filter the list of files:
files_to_convert = (f for f in os.listdir(path1) if f.endswith(".jpg"))
for filename in files_to_convert:
... process the file ...
Ensure that the listing of *.jpg are files, not subdirectories named *.jpg:...
3
Echoing all of VincentRG's points, with a little clarification and demonstration:
The only reason to wrap code in a if __name__ == '__main__' block is to keep it from being executed when your module is imported by another module. By that token, the fact that you initialize bot_choices outside of that block means that those random choices will be determined ...
3
Since you already have a nice implementation of a prime sieve, you should make use of it when checking for prime factors. Note that prime factors are needed, not all factors. I definitely would not use a recursive algorithm for this. It can fail with a stack limit and you need a lot of unreadable code just to set up the data structures.
I would use ...
Only top voted, non community-wiki answers of a minimum length are eligible
