Photomosaic task #58
Photomosaic task #58
Conversation
There was a problem hiding this comment.
Generally:
- No underscore imports, please
- More comments!
- Better docs!
- Add a README with an example!
- Try to new-line after code with comments (my preference really, but I think it looks better):
- I will review again after these fixes :)
# some comment
some_code()
# another comment
more_code()
For commits, consider rebasing. In PyCharm:
You can fixup, drop, reword commits (fixup means merge to the above one):
Keep in mind you will need to force push (so careful!!! We can do it together if it's easier / you feel safer, at the end, once the branch is ready to be merged):
setup.py
Outdated
| @@ -41,7 +41,7 @@ | |||
| "numpy", | |||
| "sklearn", | |||
| "pandas", | |||
| "opencv-python", | |||
| "opencv-python==4.5.1.48", | |||
There was a problem hiding this comment.
This is fine, but can you share the error proving this is the problem? It's a bit weird fixing the OpenCV installation version works...
There was a problem hiding this comment.
https://stackoverflow.com/questions/67837617/cant-parse-pt-sequence-item-with-index-0-has-a-wrong-type-pointpolygon-er This is the solution I have found. Probably it has something with the integer types in function draw_circles. I might check it, but downgrading this version has solved it. The problem arose while testing.
There was a problem hiding this comment.
Ok, that's what you'd normally call a dirty hack, since it doesn't actually solve the problem and only walks around it. This might prove to be a problem in the long run (for example if some future cv version introduces some really cool stuff we will want to use). I will have a look at fixing the exact error, if it's simple enough I will just push to your branch, otherwise, we will go on with this fixed-version hack for now.
There was a problem hiding this comment.
I have found that it might be the problem with parsing the integers, I am trying to refactor the code and will see if the actual parsing int(SOMETHING) would solve this
There was a problem hiding this comment.
I have found that it might be the actual problem
surface/constants/vision.py
Outdated
| @@ -1,3 +1,17 @@ | |||
| """ | |||
| Computer vision constants. | |||
| """ | |||
| import numpy as _np | |||
There was a problem hiding this comment.
Should be import numpy as np, underscore-imports are a little outdated really.
| import numpy as _np | ||
|
|
||
| # The height of the cut picture | ||
| MOSAIC_HEIGHT = 300 |
There was a problem hiding this comment.
Add whitespace, please. Also, is "The height of the cut picture" referring to the result, or the input?
surface/vision/mussels.py
Outdated
| @@ -42,7 +43,7 @@ def _gaussian_blur_smooth(mask: ndarray) -> ndarray: | |||
| return blurred | |||
|
|
|||
|
|
|||
| def _get_edge_points(mask: ndarray) -> list: | |||
| def _get_edge_points(mask) -> list: | |||
surface/vision/mussels.py
Outdated
| @@ -164,7 +165,7 @@ def count_mussels(image: ndarray) -> Tuple[int, ndarray, ndarray, ndarray, ndarr | |||
| hull_rect = _get_corner_points(points) | |||
|
|
|||
| # Draw hull rect on the original image | |||
| convex_hull: ndarray = image.copy() | |||
| convex_hull = image.copy() | |||
surface/vision/photomosaic_photo.py
Outdated
| img_cut = _cut_images(images_hsv) | ||
|
|
||
| dict_color_map = _color_detect(img_cut[:1]) | ||
| print(dict_color_map) |
surface/vision/photomosaic_photo.py
Outdated
|
|
||
| # divide the top and bottom image | ||
| bottom_index, top_index = _type_division(dict_color_map) | ||
| print("Image count", len(img_cut)) |
surface/vision/photomosaic_photo.py
Outdated
| # divide the top and bottom image | ||
| bottom_index, top_index = _type_division(dict_color_map) | ||
| print("Image count", len(img_cut)) | ||
| print(bottom_index, top_index) |
surface/vision/photomosaic_photo.py
Outdated
| for i in range(5): | ||
| # This line to be changed when we get the camera | ||
| img.append(_cv2.imread("./proper_samples/" + str(i + 1) + ".png")) | ||
| print(len(img)) |
| print("Image count", len(img_cut)) | ||
| print(bottom_index, top_index) | ||
|
|
||
| # combine the images |
There was a problem hiding this comment.
Wow I'm glad you told me what this line does :)
TheCodeSummoner
requested review from
TheCodeSummoner
and removed request for
TheCodeSummoner
MOSAIC_HEIGHT is referred to the result picture
Change in mussels.py parsing to integers by function int() instead of astype()
Added logger and got rid of print statements. Changed imports to not be underscored. Added documentation about _cut_images function, _find_rectangle, _get_contours, _combine_images, _color_detect. _get_key function is to help with combining images and getting colors of interest to match while combining.
Changed parsing to be compatible with newer versions of OpenCV
Added photomosaic task's algorithm, the order of taking photos is important (TOP -> LEFT -> FRONT -> RIGHT -> BACK). Put constants to the constants folder with vision.