# Tag Info

5

DRY. Both branches do identical re.subs. Take them out: if chars == 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(...

5

If you call regex_strip(s, ""), you will get: re.error: unterminated character set at position 0 because neither ^[] nor []$is a value 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 ... 2 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 = car print("The plural of ", word, " is ", p.plural(word)) # cars word = cars print("The singular of ",... 5 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 ... 2 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 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 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 ... 4 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. 11 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 ... 2 A tiny improvement to the part that chooses which sequence is required: if (n%2==0): prime(int(n/2)) else: fib(int(n/2)+1) Since n is already int, we can use simple integer division: if n % 2: print(fib((n+1)//2) else: print(prime(n//2)) (I've assumed the obvious improvement of making your functions pure, and moving the side-effect to ... 12 By performing recursion, you are performing a depth-first search of four-letter words. However, this task involves finding a shortest path, and shortest-path problems are generally better done using breadth-first search. With BFS, the first solution you encounter will be an optimal solution — which is not the case with DFS. 6 You could use a few tricks to implement the two sequences more efficiently, but the short version of my answer is that most significant performance improvements you could make involve some relatively advanced math, and the smaller improvements do more to improve your code's readability than its performance. Useful improvements to prime If you keep a list ... 3 Counter Great use of Counter. One thing that can be improved, is to use most_common instead of max. If edges is the Counter of all the edges in the wall, _, max_edges = edges.most_common(1) gives you the index with the most edges, and how many edges it has. Since this is the right side of the wall, you need the 2nd most common element: edges.most_common(2)[... 0 comments comments should explain why you do something, not what you do. # Append R-Squared TO R-Squared list adds nothing of value. On the contrary, it uses vertical space, and if ever you change something of the code you will need to change the coàmments as well # This function takes in a subset of a dataframe representing independent # variables (X) ... 3 successor(node, targetValue) does not necessarily return the in-order successor even if called with a node in a valid search tree and that node's value: what about search trees where a value can occur more than once? I notice no docstrings. successor() in a binary tree (I could guess that much from left and right) might be the in-order successor, ... 3 List lookup is faster than dict lookup:$ python -m timeit -s "c = {i: i for i in range(10)}" "c[3]" 10000000 loops, best of 5: 30 nsec per loop $python -m timeit -s "c = {i: i for i in range(10)}" "c[9]" 10000000 loops, best of 5: 30.2 nsec per loop$ python -m timeit -s "c = [i for i in range(10)]" "c[3]" 10000000 loops, best of 5: 26.3 nsec per loop $... 4 For idiomatic Python: You should not put spaces between the brackets and the values in function calls. You should use snake_case for variables. You've said you have concerns over finding the parent node that is the ancestor. Lets say we have the tree: A B C D Since that all left nodes are smaller than the current value in binary trees we know that \$...

4

Test cases are very limited. You might want to normalise equivalent addresses before hashing; for example, these addresses are all equivalent: user@example.org user@Example.Org "user"@example.org It's probably desirable that they hash to the same value. Simply splitting on @ is naive - it's better to split on unquoted @, or more simply, just on the last @...

9

Return early, there's no need to touch files if the current file is <5mb. DRY your code, move the "{}{}.{}".format into it's own function. Don't use comprehensions for mutations. Use an explicit for loop for that. As you want idiomatic code, you should be aware that some people discourage the use of __ as it performs name mangling. If the function is ...

-2

print("Ebay Invoice Program\n") while True: file = input("Enter 0 to Quit. Enter S to Search The File. Press ENTER to Input new Entries *Case Sensitive*: " ) if file == "0": print("Exiting Program...") break elif file=="S": keyword = input("Enter Invoice Number to Search For: ") look_in = "usb.txt" ...

6

There's a lot of repeated summing taking place in this part: (sum(row[:i + 1]) for row in wall for i in range(len(row[:-1]))) So this is summing the first i+1 elements in the row, with an upper bound that steps through all values except the last. If the row was [1, 2, 3, 4, 5], these are the sums: 1 1+2 = 3 1+2+3 = 6 1+2+3+4 = 10 The sums differ, but ...

4

You have a god-class Mozaika, you should define image mutations on another class Image. You have three mutating containers that hold the information you need. This is really really really bad. If I were an interviewer the second I see that I'd know I wouldn't want you. This is because it makes your code hard to read, and really fragile. Below is what, a ...

1

To expand on l0b0's answer: Don't chain str.replace's for the same reason you don't concatenate strings. If you need to perform multiple translations at the same time instead use str.maketrans and str.translate. >>> table = str.maketrans('ab', 'cd', 'e') >>> 'abeeba'.translate(table) 'cddc' Your code only supports Unix paths, this means ...

1

Some suggestions: I don't know whether this is community standard, but I never import * from anything. It's a quick way to get a naming collision, and I prefer being explicit about where my IDE has to look to find the implementation of something. An IDE will let you write out the name of the thing from another file and tell it to add the import statement ...

2

At first: well done, your code quality is already at a high level! Here are some remarks that you could integrate besides the class approach mentioned before. The assert statement works only if python is in debug mode It's not running when you interpret it with optimization flags, see officialdocs. It is meant as a debug tool that helps tracking down bugs. ...

1

Some suggestions: You could use str(datetime.date.today()) to get the current date. You can chain replace calls: >>> 'abba'.replace('a', 'c').replace('b', 'd') 'cddc' if len(inspect.stack()[1:]) == 0: would usually be replaced with just if inspect.stack()[1:]. Treating your variables as immutable makes the code much easier to read. For example, ...

1

Some suggestions: Do not use mutable default arguments, they will mess with your subsequent calls. There are some very rare cases where mutable default arguments make sense, but this does not look like one of those. Split generation of data from saving that data to a file. That way either method can be reused. If you also pull out the file handle to a main ...

4

The following is a profiling run of all of the methods I've seen above, with some improvements to the I/O. from io import StringIO from timeit import timeit from sys import stdout, stderr import numpy as np def justin(n): a = [] for i in range(n): a.append([]) a[i].append(1) for j in range(1, i): a[i].append(a[...

1

apart from TrebledJ's excellent review of the functional improvements of your code, here some general Python improvements get_words There is no need for this function to return a list. The rets of your code only cares that it gets an iterable, so you might as well return the set. You can also avoid the lambda expression by doing map(str.strip, word_file) ...

4

There's an even better way to generate the Pascal's triangle that only requires O(1) memory. It can even calculate a certain row directly without requiring previous rows to be computed. It works like this: the first number in each row is always 1. consider the fraction (n / 1) where n in the row index (1-based) the 2nd number is obtained by multiplying the ...

4

A search for "python sort unit test order in method definition order" got me here, and the documentation isn't particularly useful either ("Function" it says, what is it passed, what is it expected to return?) Reading the four questions linked to at the start of this one yields only three unique answers: Don't sort your test methods (not an answer). Put ...

3

TL;DR: It's unlikely to beat Python's own implementation of this which is basically as simple as decimal = int(binary, base=2) in terms of convenience, clearity, and performance. This will throw a ValueErrorin case the number cannot be converted to a binary, e.g. if the user enters something like 101010103. Welcome to the world of Python, where "...

4

A starter note, throughout my answer: FP = "functional programming". Possible Improvements Use == rather than is for value comparisons. I see if var is number in multiple places. Specifically, lines 29, 96, 109. One could argue that is reads better than ==, but the two are computationally different. (is breaks for large numbers.) Use == instead. See also: ...

6

This seems like exactly the sort of thing you would use a class for. Not just any class, but an immutable one! I'd suggest you create a class EgyptianNationalId and initialize it from a string. You could then parse out all the fields at the time of creation, and store them using read-only attributes (such as with a namedtuple). Something like this: ...

Top 50 recent answers are included