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

dials.export_bitmaps with resolution rings #2360

Merged
merged 12 commits into from
Mar 14, 2023

Conversation

dagewa
Copy link
Member

@dagewa dagewa commented Mar 6, 2023

This adds optional resolution rings to dials.export_bitmaps. Additional PHIL controls are:

show_resolution_rings = False
  .type = bool
resolution_rings {
  number = 5
    .type = int(value_min=1)
  fontsize = 30
    .type = int
}

Example with a P6M image:

dials.export_bitmaps grid_full_cbf_0005.cbf show_resolution_rings=True

image0001

This also works for multi-panel and non-coplanar detectors. For I23:

dials.export_bitmaps germ_13KeV_0001.cbf show_resolution_rings=True resolution_rings.number=10

image0001

@dagewa dagewa linked an issue Mar 6, 2023 that may be closed by this pull request
@dagewa
Copy link
Member Author

dagewa commented Mar 6, 2023

I see there are some artefacts at the image edges for the single panel case

@Baharis
Copy link
Contributor

Baharis commented Mar 6, 2023

This might be more of an esthetic choice than anything, but what would you say about putting all phil parameters in one scope, i.e.:

resolution_rings {
  show = False
    .type = bool
  number = 5
    .type = int(value_min=1)
  fontsize = 30
    .type = int
}

@dagewa
Copy link
Member Author

dagewa commented Mar 6, 2023

Yeah, I'm inclined to agree that's better 👍

@dagewa
Copy link
Member Author

dagewa commented Mar 6, 2023

Ah, I know why I did that. It's because dials.image_viewer has the parameter show_resolution_rings so I was going for consistency. But hardly anyone uses the PHIL parameters for dials.image_viewer, so that's not a very strong reason.

@Baharis
Copy link
Contributor

Baharis commented Mar 6, 2023

Ah, consistency is definitely also a great reason. I personally don't have particular interest in any version; I was just curious.

These happen when a path has gone outside a panel and then back in.
To keep the calculation simple just reject any segment with a dx or
dy distance of > 30 pixels
@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Merging #2360 (f93ef98) into main (cfe100e) will decrease coverage by 4.39%.
The diff coverage is 80.67%.

❗ Current head f93ef98 differs from pull request most recent head 49abaa0. Consider uploading reports for the commit 49abaa0 to get more accurate results

@@            Coverage Diff             @@
##             main    #2360      +/-   ##
==========================================
- Coverage   82.90%   78.51%   -4.39%     
==========================================
  Files         593      602       +9     
  Lines       68592    73173    +4581     
  Branches     9221     9935     +714     
==========================================
+ Hits        56863    57454     +591     
- Misses       9613    13599    +3986     
- Partials     2116     2120       +4     

@dagewa dagewa merged commit 73785dc into main Mar 14, 2023
@dagewa dagewa deleted the 1961-export-bitmap-with-resolution-rings branch March 14, 2023 16:53
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.

Export bitmap with resolution rings
3 participants