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

Improve docs for linop.xray subpackage #533

Merged
merged 10 commits into from
Jun 17, 2024
Merged

Improve docs for linop.xray subpackage #533

merged 10 commits into from
Jun 17, 2024

Conversation

bwohlberg
Copy link
Collaborator

@bwohlberg bwohlberg commented Jun 12, 2024

Improve docs for linop.xray subpackage. See also lanl/scico-data#49.

The effects of the changes in the built docs may be viewed here. These docs are likely to be moved or edited as part of the completion of #529.

@bwohlberg bwohlberg added the documentation Improvements or additions to documentation label Jun 12, 2024
Copy link
Contributor

@Michael-T-McCann Michael-T-McCann left a comment

Choose a reason for hiding this comment

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

Cool plots!

Gut reaction: I object to the labels "y (axis 0)" and "x (axis 1)" on the SCICO plot. The code is written so that axis 0 goes with x and axis 1 axis with y. The downside of that is that it means rows <-> x and columns <-> y.

We could add the motivation for why SCICO works this way. The idea is to think about rotation (by theta) followed by projection (onto the x-axis). Probably I could improve the Parallel2dProjector docs to make this more clear (by avoiding talking about rows and columns).

@bwohlberg
Copy link
Collaborator Author

Comments addressed.

@bwohlberg bwohlberg merged commit 81806fc into main Jun 17, 2024
18 checks passed
@bwohlberg bwohlberg deleted the brendt/xray-docs branch June 17, 2024 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants