Skip to content

CoordinateSource pixel_size handling - #1295

Merged
garrettwrong merged 7 commits into
developfrom
mrcsrcpxsz
Jul 1, 2025
Merged

CoordinateSource pixel_size handling#1295
garrettwrong merged 7 commits into
developfrom
mrcsrcpxsz

Conversation

@garrettwrong

Copy link
Copy Markdown
Collaborator

Adds pixel_size to initializer for CoordinateSources. Also add basic test for the explicit, None, and mismatch case.

@garrettwrong garrettwrong self-assigned this Jun 24, 2025
@garrettwrong garrettwrong added enhancement New feature or request cleanup labels Jun 24, 2025
@codecov

codecov Bot commented Jun 24, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.53%. Comparing base (ceb6cbd) to head (a311520).
Report is 7 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1295   +/-   ##
========================================
  Coverage    90.52%   90.53%           
========================================
  Files          132      132           
  Lines        14209    14219   +10     
========================================
+ Hits         12863    12873   +10     
  Misses        1346     1346           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@garrettwrong
garrettwrong requested a review from j-c-c June 25, 2025 15:03

@j-c-c j-c-c left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks good. Just one questions and a couple typos if you feel like changing them.

Comment thread tests/test_coordinate_source.py Outdated
Comment thread tests/test_coordinate_source.py Outdated
Comment thread src/aspire/source/coordinates.py Outdated
@garrettwrong
garrettwrong requested a review from j-c-c June 26, 2025 16:10
j-c-c
j-c-c previously approved these changes Jun 26, 2025

@j-c-c j-c-c left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@garrettwrong
garrettwrong marked this pull request as ready for review June 26, 2025 18:23
@garrettwrong
garrettwrong requested a review from janden as a code owner June 26, 2025 18:23

@janden janden left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good. Just one thing.

Comment thread src/aspire/source/coordinates.py Outdated
:param max_rows: Maximum number of particles to read. (If `None`, will attempt to load all particles)
:param B: CTF envelope decay factor
:param symmetry_group: A `SymmetryGroup` object or string corresponding to the symmetry of the molecule.
:param pixel_size: Pixel size of the images in angstroms, default `None`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should indicate what None means in this context (auto-assign from CTF if possible).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sure I changed the three occurrences to the following

        :param pixel_size: Pixel size of the images in angstroms.               
            Default `None` will attempt to infer `pixel_size` from              
            `CTFFilter` objects when available.  
            

@garrettwrong
garrettwrong requested a review from janden July 1, 2025 18:52
@garrettwrong
garrettwrong merged commit 04c9c04 into develop Jul 1, 2025
@garrettwrong
garrettwrong deleted the mrcsrcpxsz branch October 23, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants