# Tag Info

0

Using a set() is your bottleneck, memory-wise. >>> numbers = set(range(3, 10**8, 2)) >>> sys.getsizeof(numbers) 2147483872 A set of odd numbers up to 100 million is consuming 2GB of memory. When you do an operation like numbers = numbers - num_set, you'll need to have 3 sets in memory at once. One for the original set, one for the set ...

1

The problem that you observe with this program is speed, so let's look at that. Running the program, I immediately noticed that the get_permutations section was slow, and the dictionary_check section was many times faster. That immediately tells me that it's not worth looking for faster ways to do the dictionary_check until get_permutations is much faster. ...

7

Is there a way to solve this memory or time problem using my code or should use a different algorithm? The algorithm is fine for the kind of scale you're talking about. It's the implementation of the algorithm which needs optimisation. To tackle the memory issue, look at set. Given that the elements of the set are integers from a fixed range and moderately ...

5

I think your performance problems at 10**6 elements start here: for i in range(3, int(num**(1/2)) + 1 , 2): This generates a list of numbers that you then build a set of multiples from and remove those multiples from the numbers set. But this generates a set [3,5,7,9,11,13,15,...] so when you've removed all the multiples of 3 you still try and remove ...

0

After some research, I ended to a better and faster solution. Here is the code: # import packages import numpy import mahotas.polygon import shapely.geometry as shageo import cv2 import numpy as np def get_mask(dims, pts): # create a numpy array of zeros with the same dimensions of the image canvas = numpy.zeros((dims[0], dims[1]), dtype=int) ...

2

The main piece of advice I feel I can offer is to avoid using the asterix from list2BST import * This makes it much harder for other users to work out where individual functions are coming from. It is much better practice to say import list2BST and then list2BST.<function name> or from list2BST import <function_1>, <function_2> I ...

6

My current effort is an implementation around prime factorization. I am attempting to reproduce something similar to the table found here Prime factorisation is the wrong approach to build that table. The efficient way to build it is to generate a list of primes using a sieve of some kind (a good implementation of Eratosthenes is definitely good enough up ...

1

The code feedback is already done wonderfully. I'd like to write about some pointers that might be important, apart from the leet speak: The word Phoque , is French for seal. There are numerous examples of words which are perfectly meaningful in some language but if pronounced in English, might be considered offensive. You might need to do a phonetic ...

13

Some suggestions related to review performance: This code is really hard to read. As far as I can tell every single variable except for primes is abbreviated to the point where I need to hold the entire program in memory in order to reason about any part of it. Naming is really important for readability, and readability is really important for ...

2

Time will tell if your question is considered a worthy Code Review question, but till then I'ld like you to give a short review on your code nevertheless. Format function You could reduce the code duplication in the format function and make use of .format or f-strings (from Python 3.6 onwards). def sizeof_fmt_rev(num, suffix='B'): for unit in ['', 'Ki'...

2

It was hard to understand, how your code works, so I started by changing variable names to more meaningful form, also unnecessary +1, -1 operation were fixed (i - 1, 1, len(key) + 1, etc): class Solution: def findRotateSteps(self, ring, key): cumulative_lens = [[min(i, len(ring) - i) for i in range(len(ring))]] cumulative_lens.extend([[...

2

explicit_words = [ #explicit words not shown ] Instead of embedding the word list within the program, you might want to treat it as configuration, and load it from a data file. The advantage of this is that it can allow the use of alternative configurations by command-line option (e.g. use a list of French obscenities for French-language lyrics). On ...

5

You seem to make use often return 1. It would be better to use booleans to show a clear intention of what you want to return. Also, your hash_route, which you build as a dict, has a constant value that is never used, making it effectively a set, which is fine if all you care is the lowest common ancestor. I would go for: def traverse_DFS(root, ...

2

Code readability and style I would recommend you have a look at PEP 8, which is Python's official style guide. Let's introduce you to f-strings - To create an f-string, prefix the string with the letter “ f ”. The string itself can be formatted in much the same way that you would with str.format(). f-strings provide a concise and convenient way ...

7

Code readability and style Your code has nothing wrong with style (as far as I know). It seems to be (99.9999...%) PEP 8 compliant. I ran a PEP 8 checker over your code and this is what it picked up - Which basically tells you to add a space before the operator '=' here - hash_route= {} # hash_route = {} As for the missing newline at the end of the ...

3

To simplify your input you could change word == explicit_word to explicit_word in word. This can lead to the Scunthorpe problem, and so from here you can check if the word only contains prefixes and suffixes. Given the amount of common prefixes alone, it's unlikely you'd catch all variations using a blacklist alone. It would also catch gibberish/...

1

Returning early makes code not follow the arrow anti-pattern. And makes the flow of the code simpler. There's otherwise not much to comment on. class LoginSerializer(serializers.Serializer): phone = serializers.CharField() password = serializers.CharField() def validate(self, data): phone = data.get("phone", "") password = data....

3

Some suggestions: The translations strings are duplicated, and are present in code rather than as a separate text file. I've not done i18n yet, but that seems strange. I would expect a .po file containing text and translations. user_imputs should be user_inputs You mix four types of string formatting: +/+=, %, .format() and f-strings. The first two I ...

6

DRY. Both branches do identical re.subs. Take them out: if chars is None: strip_left = re.compile(r'^\s*') strip_right = re.compile(r'\s*$') else: strip_left = re.compile(r'^[' + re.escape(chars) + r']*') strip_right = re.compile(r'[' + re.escape(chars) + r']*$') s = re.sub(strip_left, "", s) s = re.sub(strip_right, "", s) return s I ...

7

If you call regex_strip(s, ""), you will get: re.error: unterminated character set at position 0 because neither ^[] nor []\$ is a valid regular expression. You could avoid this by using if not chars: instead of if chars == None:. There is no need to re.compile() your regular expressions; you aren't saving the compiled patterns anywhere for re-use. You ...

3

As mentioned in a comment by Roland Illig, I missed the interesting part of generating the correct plural forms from the singulars. Here's a module which supports Python 3 - Inflect. # Initialization import inflect p = inflect.engine() Examples - word = "torch" print(f"The plural of '{word}' is '{p.plural(word)}'.") >>> The plural of 'torch' is ...

8

I am suggesting to use fstrings and the dictionary items() method. The print(f'{value} {key}') instead of print(str(inventory[item]) + ' ' + item) is more neatly: def display_inventory(inventory): total_items = 0 print ("Inventory:") for key, value in inventory.items(): print(f'{value} {key}') total_items += value ...

15

Some suggestions: Using argparse to parse arguments is preferable to any kind of interactive input, because it means that it's much easier to automate and integrate in other scripts, you could trivially support passing multiple songs in one command, it's what any experienced shell user would expect, and if the user adds --help or doesn't pass any arguments ...

25

I recommend practicing Python 3 rather than Python 2 these days. According to PEP 8, isClean() should be is_clean(). One or more of your loops could be replaced by some use of the any() function. Note that this suggests that an is_explicit() function would be a more natural concept than an is_clean() function. I would expect song lyrics to contain a ...

21

Splitting line into words using words = line.split(" ") will only split the line on space characters. If two words are separated by a tab character, or other white space character other than a space, the split will not happen at that point, which may confuse your explicit checker. Using words = line.split() would split on spaces, tab characters, and any ...

3

I'd take a slightly different approach. Rather than include the n/2 loop in both the Fibonacci and prime code, I'd make it external and turn these two programs into simpler, infinite generators that are easier to debug: ''' Given a series in which the odd terms forms the Fibonacci series and even terms form the prime number series, this program finds the ...

1

Might want to look at LaTex. It's a lot less code and it IS possible to add comments in, so it's definetly possible to use it as both a template and a way to generate a pdf!

0

First of all you should definitely clean up your code. There are a lot of unused variables and duplicate code, likely caused by copy and paste. Since you did not provide test data, there is no chance to tell how much time would be gained simply by removing [R[M]]SE computation from the functions. Apart from that, its likely that the different funcs are the ...

3

Not a performance suggestion, but you can make use of the fact that an empty collection is Falsey. if len(s) == 0: Is functionally the same as just: if not s: And similarly, while len(s) > 0: Can be just: while s: Relevant PEP entry (search for "For sequences" under the linked heading).

3

As @Peilonrayz said - your code is well built and according to me, unexpectedly short which is certainly a good job as most people tend to write long programs for short tasks like this. But you could make it much shorter (a single line perhaps) - >>> print('\n'.join(map(''.join, zip(*grid)))) ..OO.OO.. .OOOOOOO. .OOOOOOO. ..OOOOO.. ...OOO... .......

4

You can use np.cumsum and np.minimum.accumulate (which I found from this post). Another way to look at what you want is you want the cumsum. If the value goes below zero then you want to subtract a value to get it to zero, this value is itself. This means that you just need a running minimum. This is as you've subtracted the value from an earlier value so ...

7

Data classes Since you are using the classes as immutable data containers, it would be possible to significantly cut down the amount of code you have to write to create all of them using namedtuple from the collections module: from collections import namedtuple Creator = namedtuple("Creator", ["first_name", "last_name", "email", "phone_num", ...

2

There's two steps I think you could leave out: Casting the diff as a list, you can iterate over a numpy array just fine Checking if cum_sum is smaller than zero, you can check this using max() def pos_diff_cum_sum(flow_in: np.ndarray, flow_out: np.ndarray) -> np.ndarray: sums = [] cum_sum = 0 diff = flow_in - flow_out for dd in diff: ...

2

You can make your code shorter and much faster by using stack - Stack works on the principle of $“$Last-in, first-out $”$. Also, the inbuilt functions in Python make the code short and simple. To add an item to the top of the list, i.e., to push an item, we use the append() function and to pop out an element we use the pop() function. These ...

4

Your code is pretty good. It's what I'd expect the author of the book to have wanted their readers to write. Good job! In terms of making the code more understandable/maintainable you can get the same functionality using zip, *args, ''.join and a single print. for column in zip(*grid): print(''.join(column))

4

AlexV has covered the formatting stuff. With regards to the length and efficiency of the code: You've clearly put a lot of thought into how to calculate the list of tuples requested. And you've found some good efficiencies! But you're approaching this from the wrong direction: Start with the clearest syntactically legal expression of the desired result you ...

5

You don't need to manually call f.close(), that's what the with is for. It looks like line = line.replace(line, "".join(dummy)) can just be line = "".join(dummy). It's clearer to define dummy in the if statement: This means that it's in the correct scope, meaning we don't have to look out for it being used elsewhere. It also means that you can remove dummy....

3

Your code is hard to understand, even after watching the video. Make some functions. years_doomsday would help move some of the hard to understand information to be self-contained. If you need to floor divide and get the remainder use divmod. You can check the leap year using calendar.isleap. You have a bug, if you ever enter a leap year then the non-leap ...

3

Have you actually read or at least skimmed PEP8 regarding the aspects mentioned by the style checking tool? The first error (E711) basically wants you to write if nums is None or ... instead of . You can find this at section Programming Recommendations. The second one (E702) tells you that you are using a semicolon to cram multiple lines into a single ...

5

Your Queue class should be replaced by the builtin collections.deque which offers better performances (lists .pop(0) are $\mathcal{O}(n)$ since the remainder of the list have to be shifted, but deque.popleft() is $\mathcal{O}(1)$). You should also take the habit of opening files using the with statement to avoid keeping opened file descriptors around: ...

6

I'm a little bit short on time at the moment, but I'ld like to share a few minor observations with you. Maybe I will also find time to look at performance related optimizations and expand it later. You said you would like your code to be Pythonic. With that in mind you could replace print(list(reversed(values))) by print(values[::-1]) which uses slicing (...

4

Apart from the remarks already given about returning instead of printing, and an argument for the number of doors, this code looks good. Instead of looping over the list, you can also use slicing: def check_doors_round_splice(n, num_doors=100): """Check which door is open after n rounds""" doors = [False] * num_doors for step in range(min(n, ...

4

One thing I would add would be to describe what your inputs should be, and to check if they are indeed the correct input type. For small scripts it's not that pressing, but in my experience it can make debugging much easier in the future. I've also copied @Peilonrayz suggestion because I agree with it. def check_doors_round(n, number_doors=100): """ ...

2

Named tuples are a good idea here. If you need to mutate the data you should instead use dataclasses.dataclass. But as there is no need here it's good. I'd suggest changing your nodes to contain snakes, ladders, and the start and end of the board. This is as you don't need to build the empty spaces as they are only used as distances. Since you need the ...

-1

You should use: guess = int((hborder + guess) / 2) instead of guess = round((hborder + guess) / 2) for correct computation.

5

continue # Forbid a dice-roll that lands on a snake This may prevent finding the shortest path - it's possible to imagine a board with two long ladders, where the first ladder passes the bottom of the second ladder, but a snake descends from after the top of the first ladder to before the bottom of the second ladder. Be careful: if you do follow snakes, ...

7

Don't use mutable default arguments. If you need to default to a list then default to None and then change to an empty list. Take the following example code: >>> def example_list_builder(value, list_=[]): list_.append(value) return list_ >>> example_list_builder(1) [1] >>> example_list_builder(2) [1, 2] This makes list_ ...

12

Mathematical observation Let's consider the i-th door after n rounds and see when its state changes. This boils down to considering the divisors of i smaller than n. In particular, we could try to handle them by pair (p, q) such than i = p * q. Without limitation, we can assume p <= q. If 0 < p < q < n ("usual situation"), the door will change ...

14

It would be better if you merged (index+1) % (step+1) == 0 into the preceding for loop. Whilst it's easy to understand what it means, it's even easier to understand what range(start, stop, step) means. You should return doors and print outside the function. I'd prefer to be able to specify how many doors to use. This can be a default argument. def ...

3

This is my first post for suggesting improvements so I may be way off the mark but I think you should Use docstrings in each function and preferably have an overall docstring Use a helper function (in my case find_n_term) so this module can be re-used by other programmers Use the guard if __name__ == "__main__" so others users can import the module and it ...

Top 50 recent answers are included