New answers tagged python
0
The important fact first: You are required to refactor a reasonable piece of code into an evil one. An if-elif-else-rake is very seldom the best solution. That said I'll try to give two general rules.
When testing for "a multiple of 6 (6, 12, 18, 24, 30)"
if A % 6 == 0:
is the more readable and less error prone solution compared to
if A % 2 == 0 ...
2
The following is better IMO. Duplicated "A is odd" / "A is even" strings are omitted from the output. They could be added if required.
if A % 2 == 0:
if A % 6 == 0:
msg = "a multiple of 6 (6, 12, 18, 24, 30)"
elif A % 10 == 0:
msg = "a multiple of 10 (10, 20)"
else:
msg = "a ...
3
I think that having a range is not necessary here. If you're only comparing against integer ranges, simple \$ \le \$ and \$ \ge \$ comparisons should suffice.
So, the following function is essentially the same:
def search_position(ranges, positions, window_size=10000):
for low, high, range_id in ranges:
for pos in positions:
if low &...
1
Keyerror
Did you test this code with some invalid input?
The code will fail with a keyError if the items key does not exist, since your code just assumes it exists.
You could do a try-except with the json_to_test['items'] so that you can handle the keyError.
Unspecificness
if any(isinstance(i, list) for i in json_input)
You're testing that json_input ...
3
You have a few functions with an unusual conditional structure. I might be
misunderstanding something, but I assume the player either wins or loses. In
that light, there's no purpose to the second conditional check: just use
else.
def winloss_generator(rng_v):
if rng_v <= .49: # win
return 'win'
if rng_v > .49: # ...
1
I agree with Abdur keep it simple with one class. Other things that could be improved is:
Add a data base, you could use pickle or shelve to store user names
and passwords.basic start to shelves
2)Add a button for new users right away.
3)your use of relx and rely has some overlapping side effects you might be
better off using x and y with real coordinates.
...
2
The biggest thing that stands out to me is the repetition in your code. The same large groups of variables are written out and passed around in the same order repeatedly and the same function is called over and over for different arguments. Those are signs that what you're doing should probably be simplified.
In particular the model of your config and ...
2
Your algorithm seems good, but the while loop, and lecture_hour and lecture_minute variables make your code a lot more complicated.
If we KISS then a simple algorithm is to just remove () from Lectures and iterate through it, since it is sorted.
The first lecture that is after the current time is the lecture we want.
This is nice and simple:
import datetime
...
2
When you don't care about casing, rather than repeating A-Za-z multiple times, it can be preferable to use the case-insensitive flag; this makes the pattern terser and more readable, as well as less error-prone.
It would also be good to use informative variable names; the variable name x is not very useful. Maybe call it match instead.
You can also consider ...
14
This is more "streaming" in a sense:
from heapq import nlargest
from operator import itemgetter
def top_ten_scorers(stats):
players = (dict(name=name, ppg=points/games, team=team)
for team, players in stats.items()
for name, games, points in players
if games >= 15)
return nlargest(10, players,...
5
It is possible to write those steps in a single comprehension -- sort of the Python
analogue for chaining in JavaScript or Ruby. It doesn't read too badly if you
convey the logic visually. Without that attention to code layout,
too much burden would be placed on
the readers and maintainers.
from operator import itemgetter
def top_ten_scorers(stats):
...
1
Some usual PEP8 comments:
username_clear.delete(0,END) -> username_clear.delete(0, END)
in_use.place(relx= 0.0, rely= 0.7, anchor= "sw") -> in_use.place(relx=0.0, rely=0.7, anchor="sw")
New_user should have either been NewUser for a class name or new_user in other cases
Choose meaningful names & avoid typos
Avoid typos like: ...
3
That regex will not properly validate the format of an email address. Many web sites get it wrong when validating email addresses.
According to RFC5322, the local part of an address (the part before the @) can contain one or more text strings separated by .s. The test strings can include letters, numbers, and any of these characters: ! # $ % & ' * + - / ...
1
You don't need to create a second m x n matrix to track where you've set. Since you're only interested in the index of the last one placed in each column you can use an one dimensional n-length array to track the row of the last one added and then use that directly when indexing the matrix.
Similarly, it also lets you randomly choose columns that still have ...
3
Here I provide a different approach to solve this problem more efficiently. Compared with OP's solution, the primary optimization comes in the following aspects:
Calling isin four times for each item class (Dessert, Hot, Juice, Milk) is inefficient. A better approach is to join the original DataFrame df with a Series that maps each item to a class, and then ...
2
Here's your first 65 lines (everything before main) cut down to 16 with Pandas.
Since you're new to programming watch out for when you start repeating the same lines with different variables or the same functions with different number of arguments. Those are signs what you're doing can likely be simplified.
And if you're trying to do anything with data ...
2
You have a useful review on several details already, so I'll focus on overall
design. In particular, what good is the larger class doing you? All of your
PEP8 checkers have access to self and they mutate it when errors occur. It
feels like the detailed, algorithmically grubby code of the checkers
knows far too much. What can be done to simplify their role? ...
3
When I copy&paste your code into my editor, it immediately greets me with 50 errors and warnings. To be fair, some of these are duplicates, because I have multiple linters configured. However, well over 20 of those are real. And ironically, at least 6 of those would have been reported by your own code!
PEP8 violations
Maximum line length
Blank lines (...
0
I'm assuming that you are using python 3
When I ran the code, there were some errors.
First, there was an invalid syntax in the printBoard function. Here is the fixed version:
def printBoard(board): # this prints the board
print(" | | ")
print(" ", board[0][1], " | ", board[0][1], " ...
3
This is the same approach you used, just rewritten as a oop approach per pacmaninbw with a class socketConn inheriting from websocket.WebSocketApp
Because the on_message, on_error, and on_close functions you define are so short/simple you can set them directly in the overloading init with lambda functions instead of writing out each one separately and ...
3
Welcome to code review community. Few initial thoughts, when writing python code; following PEP-8 style guide makes your code more maintainable. Which would amount to (but not limited to):
Functions and variables named in lower_snake_case.
Avoiding whitespaces in function parameters/arguments.
Moving on, you can put your websocket handling code in its own ...
7
Except under unusual circumstances, classes are things or entities -- hence the
term objects -- while functions or methods are actions or operations. You
want to name them accordingly. For that reason, Get_labels strikes me as an
oddly named class. Based on what you've shown us, I might suggest the name
Bounds as an alternative. A side benefit of that name ...
3
Style
Please run your code through a style checker such as
http://pep8online.com/
In particular your lines should not end in ; and you don't need () for if checks.
Also a better code editor would give you hints while writing code to improve and fix style errors and other mistakes. See some suggestions here https://www.guru99.com/python-ide-code-editor.html
...
3
Value Objects
The following shows what most users would consider unexpected behaviour:
from ccomplex import Complex
a = Complex(5, 4) + Complex(3)
b = a
a.re = -a.re
print(b) # "-8 + 4i"
Values are usually considered to be immutable. Since Python uses objects to represent values, and objects have identity which can be shared, the best practice ...
1
Realistically, my review would have to be "That's not gonna work, you won't get the required accuracy with such an experiment. You need a different approach".
But here's an O(D) time simulation. Instead of potentially generating already occurred birthdays over and over again, I focus just on new birthdays. That is, I shuffle all possible birthdays ...
0
This solution uses more memory, but it's quite intuitive as opposed to some of the bulkier approaches.
First, it identifies the unique sticks in the array. It creates a corresponding dictionary storing the stick's length, and the number of sticks with this length. Each unique stick is appended to a list on unique sticks, seen as keys below.
Then, the keys ...
2
My review of your current code is easy to summarize: It's too darn complicated and
makes my head hurt. But don't feel bad, because you're in great company. First
attempts are often like that – even for people who have been doing this
for a long time.
What, specifically, makes it hard to understand? Algorithmic complexity: nested loops
and conditionals, ...
0
I'm mostly answering on SO so my answer might not be following PEP8 or other guidelines but I tried to make a version of your code that is easier to see what's going on, without ifs, breaks, continues and having a smaller line count:
length = len(L2)
missing = 'Missing_Data'
index = -1
for item in L1:
key,value = item.split(' = ')
current = L2.index(...
1
if c < len(L2):
continue
else:
c = 0
break
Since continue always happens at the end of a loop, you can reverse these conditions, to make it shorter.
if c >= len(L2):
c = 0
break
now you don't need else since anything else than the break condition will continue automatically.
Higher up where you set cont = False, you could use ...
1
In addition to the comment, a few more things you can work/focus/improve on:
Avoid global variables like corona.
Since multiple functions are sharing states, or tracking changes using globals, perhaps use a class? For eg. you can have a Board class, consisting of attributes sectors (\$ 3 \times 3 \$ squares), rows (which iterates over 9 rows) and columns (...
5
Instead of grouping by the invoice on each category, I would reverse the logic.
Group per invoice, and then classify that invoice.
categories = pd.concat(
classify_invoice(data) for invoice, data in df.groupby("invoice")
)
| | 0 |
|---:|:--------------|
| 0 | OneJuice |
| 1 | OneJuice |
| 2 | Healthy |
| 3 | ...
5
Your code is pretty impressive. Many python programmers don't know how to use pandas as well as you. Your code might not look very "Pythonic", but you did a great job utilizing vectorized methods with indexing. In this answer, I include one section on Python code conventions and a second attempting to optimizing your code.
Python Code Conventions
...
0
Nice game and great work, my daughter likes it!
It worked running on Windows. On completing the puzzle I was unable to select another picture and play again. I think this can be easily implemented by adding a method play_game(self) that takes the overall control of the game and will have a quit button.
A follow up puzzle would be to hustle the picture ...
2
From the code, it looks like the criteria for saying the books match is that they have at least one matching n-gram. Given that, the code can be simplified quite a bit.
Basically, build a data structure as the book data is read in line-by-line. Each entry has the book name and the set of n-grams.
Then look for intersecting n-grams. Keep track of items that ...
3
Looks pretty good.
I see you implemented modulus in mod. It's also called absolute value, and that's the name Python uses. If you implement __abs__, then Python's abs function can use it. Then abs(Complex(3, 4)) would give you 5.0. Just like Python's own abs(3 + 4j) does.
Another useful one is __bool__, which lets you declare zero as false, as is standard in ...
2
functools.reduce is what you need here:
from functools import reduce
def make_rotate_target(m, M, J, R):
input_chain = [J] * M
input_chain[m - 1] = R
return reduce(np.kron, input_chain)
The input_chain list could be replaced with an iterable constructed from itertools.repeat and itertools.chain to save space.
from functools import reduce
from ...
3
I think the next thing to investigate is this section, inside your AnalyzedFlow class findSubnet method:
for flow in self.refNetworks.MyEntityNetwork:
if flow.MyIPv4Network.overlaps(IPv4Network(IPv4Address(subnet))):
if flow.MyIPv4Network.prefixlen > maxMask:
First, you're constructing the IPv4Network(IPv4Address(subnet)) once per ...
1
In which we defend the honor of enumerate()
Although I learned from and appreciated the write-up by Peilonrayz, I was not
convinced by all of the characterizations. Also, I had some specific questions
not covered in those benchmarks, so I explored on my own using the script
below. These notes cover a few things I learned and reframe the discussion
a bit.
...
2
Ok, I tried to rearrange it a bit:
Corrected stop words (Should only contain lowercase words)
Used Jaccard method to calculate distances
Rearranged code structure
Rewrote it in Python3 with type annotations
You should now add an argument parser and that's basically it.
As far as I understood the task, the final goal was to remove same books.
Now you can ...
2
Assuming M is non-negative you can make it clear when J and R are returned and remove some of the nested if-else's.
Since if i+1 ==m only evaluates as true if m <= M you could check that and use different for loops if it's true/false to make things slightly faster but I think that would decrease readability for not much gain
def make_rotate_target(m, M, J,...
1
You could also do the following. Compute the first grouping by id and category, sum up the values:
y = df.groupby(["id","category"])["value"].sum()
Afterwards, grab the best category according to your definition:
y.groupby("category").sum().nlargest(1)
Combining these, so that we get the full job done:
y = df.groupby(...
4
The pop(0) takes linear time. Do that differently, in O(1) time. The standard way uses index variables. See this question's answers for some more pythonic ways. Or you could merge from right to left, using pop(), and then in the end reverse() the result.
One way to do the latter:
def merge(L1, L2):
"""Merges two (already sorted) lists to ...
3
if word in matching_words:
matching_words[word] += 1
else:
matching_words[word] = 1
If you're checking if a dictionary has a key before adding to it, a defaultdict may be a better option:
from collections import defaultdict
matching_words = defaultdict(int)
matching_words[word] += 1
int returns a 0 when called without arguments, and that 0 is used ...
4
Welcome to Code Review. Nice implementation, easy to read and understand.
Optimization
There are some "expensive" operations that can be simplified. Below I commented the relevant parts:
def Exp(D):
# the method "all" takes O(D)
while all((d == 1 for d in day_list)) == False:
# O(D)
zero_ind = (i for i,v in ...
3
Firstly lets get your code to be a little cleaner:
You can use statistics.mean rather than make my_mean.
You should use a for loop rather than a while loop in monte_carlo.
You don't need to do assign n_emperer at all in the function.
Exp and D should be lower_snake_case. This is as they are functions and variables.
You should put spaces around all ...
2
Classes are useful when you want to create more than one object from the same class, or create and destroy objects while running. In this case, you just have one object.
This is not the kind of program where you need a class. If you didn't use a class, you could have the exact same program but remove every self. in your code, which makes it shorter and ...
2
Code review
You should use better variable names then a, x and y. But otherwise since your code works your code is fine.
Performance
It seems a couple of users are confused why Python has strange performance.
Enumerate vs index
This is pretty simple both enumerate and index have the same time and space complexity.
If we have a list of False and set one to ...
4
Good implementation, easy to read and understand. Few suggestions:
Nested if-else in verify_command_line makes it not straightforward to understand:
if len(self.command_line) >= 3:
if self.command_line[0].lower() in self.languages:
if self.command_line[1].lower() in self.languages+["all"]:
if self.command_line[0] != self....
1
Make your code easier to read
I don't see anything wrong with the code, except that it is sometimes a bit hard to see what is going on. You could add some comments here and there explaining what you are doing, and improve some variable names. For example, what does uf stand for?
Also avoid using bit tricks like count = -~count when count += 1 does the same ...
6
Your solutions might have to try all pairs, so you have up to quadratic runtime.
Let's use an allegedly not adequate set to keep track of the numbers we've already seen, so that for each number, we can check in constant time whether we've seen the needed partner:
def find_two_sum(numbers, target_sum):
"""
:param numbers: (list of ints) ...
Top 50 recent answers are included
