Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complete rewrite into modular package with CLI #3

Merged
merged 28 commits into from
Jun 5, 2024
Merged

Conversation

bpurinton
Copy link
Contributor

@bpurinton bpurinton commented Sep 29, 2023

Major overhaul work in progress. Starting from first principles to build modular, reusable ASP plotting functionality. Much work to do, but I welcome comments on this PR along the way. Keeping it a draft for now, but eventually a detailed code review will come.

My goal (from slack message):

Overview plots:

  • Satellite geometry plot (modified or incorporated dg_geom_plot.py script)
  • Images used (just stereo / two pair for now, but could extend to multi-view)

Bundle Adjust plots:

Stereo plots:

The figures are written into a markdown document and that markdown is converted to PDF at the end (maybe keeping the markdown around for manual editing, or just clean this up as well)

More things like metrics and descriptions of plots and exact commands that were run to output each step can be inserted into the markdown document as well. There might also be additional control flow and options for plots specific to pinhole cameras. But the framework of the command line tool would hopefully be easily re-usable and extendable.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

This commit is a squash of about 30 messy commits I made while creating a package structure for asp_plot. In those commits, I significantly restructured the code, added a command line interface, and added tests. I also added a few new notebooks and scripts for plotting and processing stereo data. I also added a few new tests for th
@bpurinton bpurinton marked this pull request as ready for review June 5, 2024 23:37
@bpurinton
Copy link
Contributor Author

This is a huge PR that would be painful to review all of. Given that, I'm going to merge this as is. I am at the point where it is time to start working on smaller features, and having everything on this one giant PR isn't conducive to that clean work.

Note: all of the old notebooks / code from the repo are preserved in the original_code/ directory

@bpurinton bpurinton merged commit 0690ca5 into main Jun 5, 2024
@bpurinton bpurinton deleted the bp-cli-script branch June 5, 2024 23:43
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.

1 participant