Skip to content

Conversation

@ClementPinard
Copy link
Contributor

This commit adds functions to write models using python, be it binary of textual
Tests have been added too
Both have been renamed for their new functionality

This commit adds functions to write models using python, be it binary of textual
Tests have been added too
Both have been renamed for their new functionality
return qvec


parser = argparse.ArgumentParser(description='Read and write COLMAP binary and text models')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to run this code when importing the model. Otherwise these arguments will be injected in other scripts importing this module. Can you move it to the main function?

@ahojnnes
Copy link
Contributor

ahojnnes commented Dec 9, 2019

Looks great, left one minor comment. Thanks!

This commit also unifies the notation between the binary and text writers
Finally, it now longers takes the key of the dictionnary for id to let the user use they key dict they want
@ClementPinard
Copy link
Contributor Author

ClementPinard commented Dec 9, 2019

Hi thanks for you comment !

I updated my code with several improvements in addition to your suggestion :

  • Unifying notations
  • not using the dictionnary key to get the id, but the object .id attribute, so that the function caller can have any key-dict system they want

Clément

@ahojnnes ahojnnes merged commit 0dce1db into colmap:dev Dec 9, 2019
@ahojnnes
Copy link
Contributor

ahojnnes commented Dec 9, 2019

Looks great, thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants