3
\$\begingroup\$

The following source code is a solution to the Marching Square problem.

The explanation of using random numbers in ambiguous cases can be found here.

import numpy as np
from PIL import Image, ImageDraw

im = Image.new('RGB', (500, 300), (128, 128, 128))
draw = ImageDraw.Draw(im)

class Square():
    A = [0,0];
    B = [0,0];
    C = [0,0];
    D = [0,0];
    A_data = 0.0;
    B_data = 0.0;
    C_data = 0.0;
    D_data = 0.0;

    def GetCaseId(self, threshold):
        caseId = 0;
        if (self.A_data >= threshold):
            caseId |= 1;
        if (self.B_data >= threshold):
            caseId |= 2;
        if (self.C_data >= threshold):
            caseId |= 4;
        if (self.D_data >= threshold):
            caseId |= 8;
        return caseId;

    def GetLines(self, Threshold):
        linesList = [];
        
        caseId = self.GetCaseId(Threshold);
    
        if (caseId == 0):
            pass;
        if (caseId == 15) :
            pass;
    
        if ((caseId == 1) or (caseId == 14)):
            pX = self.B[0] + (self.A[0] - self.B[0]) * ((1 - self.B_data) / (self.A_data - self.B_data));
            pY = self.B[1];
            qX = self.D[0];
            qY = self.D[1] + (self.A[1] - self.D[1]) * ((1 - self.D_data) / (self.A_data - self.D_data));
    
            line = (pX, pY, qX, qY);
    
            linesList.append(line);
    
        if ((caseId == 2) or (caseId == 13)):
            pX = self.A[0] + (self.B[0] - self.A[0]) * ((1 - self.A_data) / (self.B_data - self.A_data));
            pY = self.A[1];
            qX = self.C[0];
            qY = self.C[1] + (self.B[1] - self.C[1]) * ((1 - self.C_data) / (self.B_data - self.C_data));
    
            line = (pX, pY, qX, qY);
    
            linesList.append(line);
    
        if ((caseId == 3) or (caseId == 12)):
            pX = self.A[0];
            pY = self.A[1] + (self.D[1] - self.A[1]) * ((1 - self.A_data) / (self.D_data - self.A_data));
            qX = self.C[0];
            qY = self.C[1] + (self.B[1] - self.C[1]) * ((1 - self.C_data) / (self.B_data - self.C_data));
    
            line = (pX, pY, qX, qY);
    
            linesList.append(line);
    
        if ((caseId == 4) or (caseId == 11)):
            pX = self.D[0] + (self.C[0] - self.D[0]) * ((1 - self.D_data) / (self.C_data - self.D_data));
            pY = self.D[1];
            qX = self.B[0];
            qY = self.B[1] + (self.C[1] - self.B[1]) * ((1 - self.B_data) / (self.C_data - self.B_data));
    
            line = (pX, pY, qX, qY);
    
            linesList.append(line);
    
        if ((caseId == 6) or (caseId == 9)):
            pX = self.A[0] + (self.B[0] - self.A[0]) * ((1 - self.A_data) / (self.B_data - self.A_data));
            pY = self.A[1];
            qX = self.C[0] + (self.D[0] - self.C[0]) * ((1 - self.C_data) / (self.D_data - self.C_data));
            qY = self.C[1];

            line = (pX, pY, qX, qY);
    
            linesList.append(line);
    
        if ((caseId == 7) or (caseId == 8)):
            pX = self.C[0] + (self.D[0] - self.C[0]) * ((1 - self.C_data) / (self.D_data - self.C_data));
            pY = self.C[1];
            qX = self.A[0];
            qY = self.A[1] + (self.D[1] - self.A[1]) * ((1 - self.A_data) / (self.D_data - self.A_data));

            line = (pX, pY, qX, qY);
    
            linesList.append(line);
    
        if (caseId == 5):
            pX1 = self.A[0] + (self.B[0] - self.A[0]) * ((1 - self.A_data) / (self.B_data - self.A_data));
            pY1 = self.A[1];
            qX1 = self.C[0];
            qY1 = self.C[1] + (self.B[1] - self.C[1]) * ((1 - self.C_data) / (self.B_data - self.C_data));

            line1 = (pX1, pY1, qX1, qY1);
    
            pX2 = self.C[0] + (self.D[0] - self.C[0]) * ((1 - self.C_data) / (self.D_data - self.C_data));
            pY2 = self.C[1];
            qX2 = self.A[0];
            qY2 = self.A[1] + (self.D[1] - self.A[1]) * ((1 - self.A_data) / (self.D_data - self.A_data));

            line2 = (pX2, pY2, qX2, qY2);
    
            linesList.append(line1);
            linesList.append(line2);
    
        if (caseId == 10):
            pX1 = self.B[0] + (self.A[0] - self.B[0]) * ((1 - self.B_data) / (self.A_data - self.B_data));
            pY1 = self.B[1];
            qX1 = self.D[0];
            qY1 = self.D[1] + (self.A[1] - self.D[1]) * ((1 - self.D_data) / (self.A_data - self.D_data));

            line1 = (pX1, pY1, qX1, qY1);
    
            pX2 = self.D[0] + (self.C[0] - self.D[0]) * ((1 - self.D_data) / (self.C_data - self.D_data));
            pY2 = self.D[1];
            qX2 = self.B[0];
            qY2 = self.B[1] + (self.C[1] - self.B[1]) * ((1 - self.B_data) / (self.C_data - self.B_data));

            line2 = (pX2, pY2, qX2, qY2);
    
            linesList.append(line1);
            linesList.append(line2);
    
        return linesList;


def marching_square(xVector, yVector, Data, threshold):
    linesList = [];

    Height = Data.shape[0];#rows
    Width = Data.shape[1];#cols

    if ((Width == len(xVector)) and (Height == len(yVector))):
        squares = np.full((Height-1, Width-1), Square())

        sqHeight = squares.shape[0];#rows count
        sqWidth = squares.shape[1];#cols count

        for j in range(sqHeight):#rows
            for i in range(sqWidth):#cols
                a = Data[j + 1, i];
                b = Data[j + 1, i + 1];
                c = Data[j, i + 1];
                d = Data[j, i];

                squares[j,i].A_data = a;
                squares[j,i].B_data = b;
                squares[j,i].C_data = c;
                squares[j,i].D_data = d;

                A = [xVector[i], yVector[j + 1]];
                B = [xVector[i + 1], yVector[j + 1]];
                C = [xVector[i + 1], yVector[j]];
                D = [xVector[i], yVector[j]];

                squares[j,i].A = A;
                squares[j,i].B = B;
                squares[j,i].C = C;
                squares[j,i].D = D;

                list = squares[j,i].GetLines(threshold);
                
                linesList = linesList + list;
    else:
        raise AssertionError;
    
    return [linesList];

def main():
    example = np.array([
                        [ 0,0, 0, 0, 0, 0,0,0,0,0,0],
                        [ 0,0, 1, 1, 1, 0,0,1,1,1,0 ],
                        [ 0,1, 0, 0, 0, 1,1,0,0,0,1 ],
                        [ 0,0, 1, 0, 1, 0,0,1,0,0,1 ],
                        [ 0,0, 0, 1, 0, 0,0,0,1,1,0 ]
                    ]);

    x = [0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100];
    y = [0, 10, 20, 30, 40];

    collection = marching_square(x, y, example, 1);
    
    for ln in collection:
        for toup in ln:
            draw.line(toup, fill=(255, 255, 0), width=5)
            
    im.save('output.jpg', quality=95)
    
main()        

Can someone kindly review this?


\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

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

Repetition

A_data = 0.0;
B_data = 0.0;
C_data = 0.0;
D_data = 0.0;

Since these are built-ins "primitives" This can be written as A_data = B_data = C_data = D_data = 0.0;

However, don't do this with the lists A B C and D above, since they would then all refer to the same list object.

Repetition 2

if (caseId == 0):
    pass;
if (caseId == 15) :
    pass;

Python does not need parenthesis for if, and it would be more natural to use or like this

if caseId == 0 or caseId == 15:
    pass

But when the same value is compared with several, a list check is even shorter (better the more values you compare to)

if caseId in [0, 15]:
    pass

The same applies to your other ifs

if ((caseId == 1) or (caseId == 14)):

Can be rewritten into

if caseId in [1, 14]:

Repetition 3

 if ((caseId == 1) or (caseId == 14)):
            pX = self.B[0] + (self.A[0] - self.B[0]) * ((1 - self.B_data) / (self.A_data - self.B_data));
            pY = self.B[1];
            qX = self.D[0];
            qY = self.D[1] + (self.A[1] - self.D[1]) * ((1 - self.D_data) / (self.A_data - self.D_data));

You have so much repetition in your many cases here that there is surely possible refactorings that can be made to shorten this code and make it simpler, but it would take more time than I have to look closely through it.

Update

line = (pX, pY, qX, qY);
linesList.append(line);

In each case you're appending the line variable after creating it, but not doing anything else with the line. So you don't need to use two lines of code for this, just append the expression directly.

linesList.append((pX, pY, qX, qY))

But there is more.

All these ifs are mutually exclusive. You only ever hit one of them and you don't use linesList for anything further, so it makes more sense and makes the code clearer to just return the linesList right away in each case. By doing an early return, you're separating each of the cases and making it clear that I don't need to see the rest of the cases once I hit one of them.

This also means that linesList, just like line, is an unnecessary variable. We can just return the expression without either of those variables.

return [(pX, pY, qX, qY)]
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.