search

iceberg-project / Penguins

MIT License
3 stars 3 forks source link

Fix1 #4

Closed hieulem closed 5 years ago

hieulem commented 5 years ago

reorganize the code's structure

bspitzbart commented 5 years ago

Tiling and merging directories are empty. They can be removed or png2patches and patches2png can be moved there. Any preference @iparask?

This pull request is only addressing the code organization, but I notice some hard-coded paths in predict.py that will have to be parameterized.

iparask commented 5 years ago

Hello @bspitzbart. I would prefer if we used tilling and merging. @lmhieu612 well done! It seems okay! One comment since this is a big pull request. Can you add doc strings to your classes at least. Slowly, we need doc strings in all classes and methods, that explain what they do, but let's start with classes first. Thank you! As soon as this is done, please ping me and I'll have another look!

iparask commented 5 years ago

Hello @lmhieu612 @bspitzbart I'm trying to review this PR and it is very difficult to understand the purpose of each class and what it is supposed to do. May I ask that doc strings with a couple of sentences are added at each class?

Also @lee212 can you add the following header:

"""
Author: Your Name
License: MIT
Copyright: 2018-2019
"""

as Bento and I have done in the Seals use case? It will be very helpful when we start merging things to a single framework.

hieulem commented 5 years ago

@iparask @bspitzbart I added codes for our CVPR submission and some descriptions for classes.