Seite 1 von 1

Falscher Rückgabetyp für Klasse

Verfasst: Freitag 12. Juni 2020, 10:47
von DR88
Hallo zusammen,
ich versuche gerade meine Skript schöner zu machen. Hierfür habe ich eine Klasse geschrieben.
Die Methode create_bounding_geom() wird über object.create_bounding_geom() aufgerufen und lierfert ein shapely Poygon als Rückgabewert.
Als nächstes möchte ich die Methode filter_for_points() über object.filter_for_points() aufrufen und erhalte folgende Fehlermeldung:

Code: Alles auswählen

class PointCloudPreprocessing:

        def __init__(self, angle_against_true_north,rotation_direction,offset_XY,bbox_rotate=None):
          
            self.__angle_against_true_north = angle_against_true_north
            self.__rotation_direction= rotation_direction
            self.__offset_XY = offset_XY
            self.__bbox_rotate = bbox_rotate

Code: Alles auswählen

AttributeError: 'Polygon' object has no attribute 'filter_for_points'
Ich verstehe warum das nicht geht. Ich möchte gern das der Rückgabewert ein Klassen-Objekt ist und habe deshalb folgendes probiert:

Code: Alles auswählen

        def create_bounding_geom(self,df,index,bauteil):


                        x_max,x_min = df["Max_X"][index],df["Min_X"][index]
                        #print(x_max,x_min)
                        y_max,y_min = df["Max_Y"][index],df["Min_Y"][index]
                        #print(y_max,y_min)

                        #create Point()
                        p_max = Point(x_max,y_max)
                        p_min = Point(x_min,y_min)

                        #create Polygon and rotate 
                        poly = Polygon([p_max,p_min,p_max])
                        poly_rotate = affinity.rotate(poly,self.__angle_against_true_north*(-1),"center",use_radians=False)

                        #create box from rotated geometry
                        p1 = Point(poly_rotate.bounds[0],poly_rotate.bounds[1])
                        p2 = Point(poly_rotate.bounds[2],poly_rotate.bounds[3])
                        #print(p1.distance(p2))

                        if bauteil == "Stützen":
                          bbox = box(minx=p1.x-offsetXY_Stütze[0],miny=p1.y-offsetXY_Stütze[1],maxx=p2.x+offsetXY_Stütze[0],maxy=p2.y+offsetXY_Stütze[1])
                          print("Stütze")

                        else:
                          bbox = box(minx=p1.x,miny=p1.y,maxx=p2.x,maxy=p2.y)
                          print("Cropping Baufeld without Offset")
                          
                        #rotate back the geometry to crop oriented PointCloud    
                        self.__bbox_rotate = affinity.rotate(bbox,angle_against_true_north,"center") 
                        return self.__bbox_rotate
Meine Frage: Wie erhalte ich als Rückgabewert ein PointCloudPreprocessing-Object.

Wäre super wenn mir jeamnd da weiterhelfen könnte.

Gruß

Re: Falscher Rückgabetyp für Klasse

Verfasst: Freitag 12. Juni 2020, 12:25
von peterpy
Hallo DR88,
ich versteh nicht ganz was Du meinst.
Wird der AttributeError zuerst ausgelöst und dann bricht der Interpreter ab?
Oder bekommst Du self.__bbox_rotate als Rückgabe und das ist kein PointCloudPreprocessing-Object?

Jedenfalls ist dein Code mit den langen Zeilen und den übergrossen Einrückungen nicht gut zu lesen.
Einrückungen sollten aus vier, nicht acht, Leerzeichen bestehen und die Zeile sollte nicht länger als 80 Zeichen beinhalten.
Gruss
Peter

Re: Falscher Rückgabetyp für Klasse

Verfasst: Freitag 12. Juni 2020, 17:11
von __blackjack__
@DR88: Ob eine Klasse das jetzt schöner macht. Der Name ist schon mal nicht gut, denn Klassen repräsentieren Dinge, während `PointCloudPreprocessing` eine Tätigkeit beschreibt. Das wäre also eher ein `PointCloudPreprocessor`, sofern die Klasse das tatsächlich repräsentiert.

`offset_XY` sollte `offset_xy` heissen. Namen werdne in Python klein_mit_unterstrichen geschrieben. Ausnahmen sind Konstanten (KOMPLETT_GROSS) und Klassen (MixedCase).

Die doppelten führenden Unterstriche bei den Attributen sollte einfache führende Unterstriche sein.

Funktionen und Methoden bekommen alles was sie ausser Konstanten benötigen als Argumente übergeben. Die `create_bounding_geom()`-Methode greift auf ”magische” Weise auf `offsetXY_Stütze` und `angle_against_true_north` aus der ”Umgebung” zu. Das heisst das ist anscheinend beides etwas das auf Modulebene existiert, wo es nicht hingehört, es sei denn es sind Konstanten, dann sind die Namen aber falsch geschrieben. Ich gehe jetzt mal davon aus, dass `offsetXY_Stütze` eine Konstante ist und bei `angle_against_true_north` eigentlich das Attribut vom `PointCloudPreprocessor` gemeint war.

`df` ist kein guter Name. Der ist für generische Beispiele gut, oder wenn man interaktiv in einer Shell was macht, aber nicht in einem Programm. Namen sollen dem Leser verraten was der Wert dahinter im Kontext des Programms bedeutet. Und an dieser Stelle kann man den Namen noch nicht mal erraten, weil man eben nicht genau weiss was das Programm in diesem `DataFrame` speichert. (Der Typ ist das einzige was man raten kann.)

Da sind noch so einige Abkürzungen. Statt `p_min` und `p_max` wäre beispielsweise `bottom_left_point` und `top_right_point` wesentlich anschaulicher und verständlicher. Wenn man `polygon` meint, sollte man nicht nur `poly` schreiben. Und `poly_rotate` ist eine Tätigkeit, wäre also eher ein Funktions- oder Methodenname. Es ist aber ein `rotated_polygon`.

Der Rückgabewert der Methode wird an das Objekt gebunden. Das macht man nicht. Wozu soll das hier gut sein? Unterm Strich wird in der Methode dann nur ein Attribut von dem Objekt verwendet. Das ist IMHO zumindest mal ein „code smell“.

Die Methode macht irgendwie auch komische Sachen. Erst wird ein eigenartiges Polygon mit der Fläche 0 erzeugt das rotiert wird, interessant ist am Ende aber nur die „bounding box“ davon. Warum ist das dann ein Polygon? Ein `MultiPoint`-Objekt mit nur zwei Punkten hätte es auch getan.

Dann wird *nur* in dem Fall das es sich bei dem Bauteil um Stützen handelt die rotierte „bounding box“ etwas grösser gemacht, und dann wird die „bounding box“ wieder zurück rotiert. Das heisst für alle Bauteile *ausser* Stützen wird einmal hin und einmal zurück rotiert, um bei der gleichen „bounding box“ anzukommen, die man auch gleich aus den beiden Eckpunkten hätte erstellen können.

Ungetestet:

Code: Alles auswählen

XY_OFFSET_STUETZE = (42, 23)


class PointCloudPreprocessor:
    def __init__(
        self,
        angle_against_true_north,
        rotation_direction,
        xy_offset,
        bbox_rotate=None,
    ):
        self._angle_against_true_north = angle_against_true_north
        self._rotation_direction = rotation_direction
        self._xy_offset = xy_offset
        self._bbox_rotate = bbox_rotate

    def create_bounding_box(
        self,
        i_need_a_way_better_name_dammit,
        what_forking_index,
        bauteil_typ_name,
    ):
        bounding_box = box(
            i_need_a_way_better_name_dammit["Min_X"][what_forking_index],
            i_need_a_way_better_name_dammit["Min_Y"][what_forking_index],
            i_need_a_way_better_name_dammit["Max_X"][what_forking_index],
            i_need_a_way_better_name_dammit["Max_Y"][what_forking_index],
        )

        if bauteil_typ_name == "Stützen":
            bounding_box = affinity.rotate(
                bounding_box, -self._angle_against_true_north,
            )
            lower_left_point = affinity.translate(
                Point(bounding_box.bounds[:2]),
                -XY_OFFSET_STUETZE[0],
                -XY_OFFSET_STUETZE[1],
            )
            upper_right_point = affinity.translate(
                Point(bounding_box.bounds[2:]),
                XY_OFFSET_STUETZE[0],
                XY_OFFSET_STUETZE[1],
            )
            bounding_box = affinity.rotate(
                box(
                    lower_left_point.x,
                    lower_left_point.y,
                    upper_right_point.x,
                    upper_right_point.y,
                ),
                self._angle_against_true_north,
            )
            print("Stütze")
        else:
            print("Cropping Baufeld without offset")

        return bounding_box