dkriegner / micro-flakes

https://dkriegner.github.io/micro-flakes/
GNU General Public License v3.0
0 stars 0 forks source link

object oriented programming of Image and Object #6

Closed dkriegner closed 12 months ago

dkriegner commented 1 year ago

I still believe the code will be easier when we make it object oriented.

ji-ze commented 1 year ago

I've just uploaded a new version of the script. It is object-oriented.

dkriegner commented 1 year ago

good. this is already a big improvement. I have more suggestions. But for this it would maybe be good to meet in person next week so that I do not propose things which are counter productive.

My thoughts are the following: I would want to structure the object into the logical entities we humans think about. From the logic I see your code works on an image. on the image it identifies objects. objects can then be filtered based on various properties and output into an excel table.

So I think there are at least three logically distinct (but of course interlinked) things to be python objects:

  1. an Image. an Image has a filename and some file-id properties. It might need methods to open/close the file and identify objects on the image. identified objects should be stored in a list of objects which should be a property of an image. The image can be created from a filename or from the webcam. So also the take_webcam_image function can be absorbed here. Likely on the Image object we will also need some "filter" methods to return subsets of the identified objects based on user input parameters (e.g. size, ...)
  2. an Object. These represent the objects you find on an image. They need to be linked to the image they belong to. Not sure if here one should store a link to the image object or the filename. Other properties will be position, size, thickness(?), opaqueness, and whatever other properties. The object should also be able to generate the small thumbnail image you put in the excel table.
  3. the excel table-object: I think this should be distinct from the other two since the "preparetable" function you have is independent of the image and object list. later there can be methods which output a list of objects into the table (or a function which outputs objects from various Image objects).

The goal should be that any of the methods you need for these objects is not much longer than 20 lines. This will allow that a new user/programmer can quickly understand what is going on. If a function needs to be much longer likely it should be split in subfunctions. lets discuss this.

ji-ze commented 1 year ago

I have a problem with the methode marketable(). The output is:

C:\ProgramData\miniconda3\python.exe C:\Users\jirka\Documents\Test\Detector\main.py 
Welcome in software to automatics detect object in microscope.
For default value write "d".

Working path: C:\Users\jirka\Documents\Test
Do you change the path? [y/N]:
You can upload existing photo or take a new photo by webcam.
Do you upload existing photo? [Y/n]: 
Files in input:
['calibration.jpg', 'Samle1.jpg']
Write name of photo from microscope.
input/Samle1.jpg
Do you want to get image output after 1st iteration?
Do you want to get image output after 1st iteration? [y/N]:
Write minimal area of edge of object in um^2. Smaller object will be deleted [42.4]: 
Write sensitivity of script on objects in dark field. Script will mark all pixels with RGB values bigger than the user's input. [40]: 
The first iteration:
The photo has been opened.
changing gamma and contrast of the original photo
marking non-black area
finding object area
finding whole objects
deleting too small object
The second iteration:
processing of 18
processed: 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, Traceback (most recent call last):
  File "C:\Users\jirka\Documents\Test\Detector\main.py", line 182, in <module>
  File "C:\Users\jirka\Documents\Test\Detector\main.py", line 178, in main

  File "C:\Users\jirka\Documents\Test\Detector\find_objects.py", line 179, in maketeble
    sheet[f"B{index + 1}"] = globals()[f"flakes{index}"].centre[0] * self.calibration
                             ~~~~~~~~~^^^^^^^^^^^^^^^^^^
KeyError: 'flakes1'

Process finished with exit code 1

More write later

dkriegner commented 1 year ago

I can not see this in the code. so I guess you did not upload it yet.

I am not sure what your error is, but the use of globals() seems to me should not be required. I believe its a sign that we did not design the datastructures right yet. We should look at it.

ji-ze commented 1 year ago

Hi, I'm sorry. The code was only committed local. Now, it's on GitHub. I've solved it, but it doesn't look good. Look at maketable() at the end of main.py

dkriegner commented 1 year ago

Lets look at this together tomorrow. I must omit I do not understand why in this method the images have to be opened.

Otherwise the maketable function in find_object.py is not so bad. There are few long equations which likely should be split out of this function. So long equations usually do something somehow sophisticated which deserves some documentation. In such case I usually prefer to have a dedicated method with a docstring. If the method should be called externally mark is a private by a underscore in the method name (def _mymethod(...)).

ji-ze commented 1 year ago

I have a problem with your pseudo-code. There are flake.pos_x, flake.pos_y, ... at the 471st line in find_objects.py (on Nov 12). It doesn't work. I don't understand this expression. The main problem is that I don't understand how Flake1, 2, ..., are stored. They are stored by https://github.com/dkriegner/micro-flakes/blob/main/Detector/find_objects.py#L77 You wrote 1st class is a list, not Flake

ji-ze commented 1 year ago

I've solved it. Now, the code works. You can see it. Comments miss only.

dkriegner commented 1 year ago

The code looks already much better! I like that the main code is not below 100 lines. good work!

One small comment:

https://github.com/dkriegner/micro-flakes/blob/eb64acd39ab0a1604671c456dbf6b77edfd59718/Detector/find_objects.py#L467-L471

Here I would suggest to use a more pythonic code:

        for index, flake in enumerate(objects):
            # fill Excel table
            sheet[f"A{index + 2}"] = index
            sheet[f"B{index + 2}"] = flake.pos_x
            sheet[f"C{index + 2}"] = flake.pos_y

in this way you avoid to always have to write image[index]. I also do not understand why the _generate_object_table(self, objects, image) method has the objects and image arguments. I believe that the image argument should be sufficient. I would say that image.marked_objects should not be needed to be accessed since all should be stored in the list?``

I'll try to have a more detailed look and add more issues soon

ji-ze commented 1 year ago

The truth! I've just rewritten it.

ji-ze commented 12 months ago

Is it open?