Colmap Instructions and Scripts #807
Conversation
| rows = self.execute("SELECT * FROM cameras") | ||
| for r in rows: | ||
| cameras.append(r) | ||
| return cameras |
There was a problem hiding this comment.
Unless I missed something, the one-liner return list(self.execute("SELECT * FROM camera")) would do the same thing.
Some of these other methods look like they could be simplified in a similar way.
| paths = [] | ||
| for img in images: | ||
| if type(img) is dict: | ||
| for (d, i) in images.items(): |
| for p in parts[:-2]: | ||
| if p not in d: | ||
| d[p] = dict() | ||
| d = d[p] |
There was a problem hiding this comment.
The for loop body could be written as a one-liner: d = d.setdefault(p, {})
| # builds nested dictionary of image folder structure | ||
| def list_images(self): | ||
| image_list = self.db.images() | ||
| images = dict() |
There was a problem hiding this comment.
Equivalent but more idiomatic to write {} instead of dict() and [] instead of list(). Comes up in several places.
| def images_to_paths(self, images): | ||
| paths = [] | ||
| for img in images: | ||
| if type(img) is dict: |
There was a problem hiding this comment.
if isinstance(img, dict):
It's considered bad form to check the type of x by comparing type(x) to something. You generally should use isinstance(x, ...), which is aware of inheritance, etc.
| matches += self.db.num_matches(img1.id, img2.id) | ||
| image_choices.append((img2.name, matches)) | ||
| best_matches = heapq.nlargest(10, image_choices, lambda x: x[1]) | ||
| images = list(map(lambda x: x.name, model1.images.values())) |
There was a problem hiding this comment.
images = [x.name for x in model1.images.values()]
| image_choices.append((img2.name, matches)) | ||
| best_matches = heapq.nlargest(10, image_choices, lambda x: x[1]) | ||
| images = list(map(lambda x: x.name, model1.images.values())) | ||
| images.extend(list(map(lambda x: x[0], best_matches))) |
There was a problem hiding this comment.
images += [x[0] for x in best_matches]
| def get_best_overlap_images(self, cur_images, subdirs): | ||
| best = None | ||
| best_matches = 0 | ||
| for i in range(len(subdirs)): |
There was a problem hiding this comment.
for i, subdir in enumerate(subdirs):
Then you can use subdir in place of subdirs[i] below.
| print( | ||
| "Building initial map %s from %s with %d images..." | ||
| % (str(map_num), data_source, len(images)) | ||
| ) |
There was a problem hiding this comment.
print(f"Building initial map {map_num} from {data_source} with {len(images)} images...")
Since we can now rely on being in Python 3... I find f-strings more readable than the old-style string interpolation. This could apply in lots of places.
If sticking with string interpolation:
- I find it's rare that I need to use any formatter other than
%s, which works by implicitly callingstr()on its argument. (This makes sense for just about any object type. No need to remember to use%dforintor whatever.) - When using
%s, it's redundant to explicitly callstr()on your argument.
| ) | ||
|
|
||
| def create_leaf_models(self, images, subdir="leaf"): | ||
| if type(images) is dict: |
|
Ah, one more general comment is that |
Readme with instructions and shortcomings, scripts for trying to do fancier things that I wasn't able to get fully working in time.