Finding students who appear on a list

Question

I'm new in Python and consequently I am searching for a specific solution for code development. I already asked some pythonic question today and want to present my whole problem. I have found some solution, but I really have interest to develop easier and clearer code in Python, so all improvements are welcome.

For example, in some school, the students are ordered to some of 3 different classes. Now I have some set of graduated students and want to know, how many graduated students are in some class.

I create next code:

physic = ['id_1', 'id_2', 'id_4']
chemistry = ['id_5', 'id_7']
math = ['id_3', 'id_8', 'id_9']

graduatedPhysic = []
graduatedChemistry = []
graduatedMath = []

classes = [physic, chemistry, math]
graduated = [graduatedPhysic, graduatedChemistry, graduatedMath]

Every class has the different number of students (given by students IDs) and gratuated Classes are at the first empty. I combine it also to list of lists.

Then I use my code:

graduatedlist = ['id_2', 'id_5', 'id_7']

for stud in graduatedlist:
    for pos1 in range(len(graduated)):
        for pos2 in range(len(classes[pos1])):
            if stud.find(classes[pos1][pos2]) != -1:
                graduated[pos1].append((stud))

This code works, but does not look great... I would be very grateful for comments! For example, I tried to implement the idea with itertools.product():

for item_1, item_2 in itertools.product(range(len(graduated)), range(len(classes[item_1])):

I have a problem with a declaration of item_1.

I have also a feeling that the lists isn't the best solution for implementation, but for DataFrame I need the column of similar length and every class can have the different number of the students.

For the special interest are two points:

1. How I really can improve my for-loop?
2. What I can use instead of list declaration?

Answer 1

Re-organising the logic

As a rule of thumb, there is almost no need for range(len(x)) in Python. Thus :

for stud in graduatedlist:
    for pos1 in range(len(graduated)):
        for pos2 in range(len(classes[pos1])):
            if stud.find(classes[pos1][pos2]) != -1:
                graduated[pos1].append((stud))

definitly looks wrong and can easily be improved :

for stud in graduatedlist:
    for pos1, grad_list in enumerate(graduated):
        for stud2 in classes[pos1]:
            if stud.find(stud2) != -1:
                grad_list.append(stud)

Now, instead of iterating over graduatedlist, it might make more sense to iterate over the classes we are considering and then, for each class, consider the student and for each student, check if he has graduated.

for pos1, grad_list in enumerate(graduated):
    for stud in classes[pos1]:
        if stud in graduatedlist:
            grad_list.append(stud)

Even better, you don't need to prepopulate graduated = [graduatedPhysic, graduatedChemistry, graduatedMath] : it makes things akward as you have to deal with indices. You could just add elements to the list as you go :

graduated = []

for c in classes:
    graduated_class = []
    for stud in c:
        if stud in graduatedlist:
            graduated_class.append(stud)
    graduated.append(graduated_class)

Making things more beautiful

Now, in Python, this looks typically like something can be written in a more concise way using list comprehension :

graduated = []

for c in classes:
    graduated_class = [stud for stud in c if stud in graduatedlist]
    graduated.append(graduated_class)

Or in a more concise way :

graduated = []
for c in classes:
    graduated.append([stud for stud in c if stud in graduatedlist])

And here we have the same pattern again :

graduated = [[stud for stud in c if stud in graduatedlist] for c in classes]

Using the right tool/data structure

Now, things can probably be done in a much smarter way using the right data structure. You want to quickly change if some element belongs or not to something. sets is what you should be using and you want to compute intersections.

physic = {'id_1', 'id_2', 'id_4'}
chemistry = {'id_5', 'id_7'}
math = {'id_3', 'id_8', 'id_9'}
graduatedlist = {'id_2', 'id_5', 'id_7'}

classes = [physic, chemistry, math]
graduated = [c.intersection(graduatedlist) for c in classes]

Or more succintly :

graduated = [c & graduatedlist for c in classes]

Answer 2

I would strongly recommend creating a class.

class Student:
    def __init__(self, id, graduated):
        self.id = id
        self.graduated = graduated

Then your problem is easier.

graduatedchemistry = [student for student in chemistry if student.graduated]

To cleanly get graduates from all classes then you can make a function

def getgraduates(students):
    return [student for student in students if student.graduated]

graduatedchemistry = getgraduates(chemistry)
graduatedmath = getgraduates(math)