New Coalignment API by nabobalis · Pull Request #293 · sunpy/sunkit-image

nabobalis · 2025-11-18T19:15:12Z

Since I lack write access now, I copied #207 here.

This should be squash merged

Fixes #302 by removing it. I didn't fix anything with anything else.

few fixes in refs fixing the refs

importing funcs testing import i think final final nail explicit imports

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

lint

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

wtbarnes · 2026-02-12T06:10:13Z

I do not understand why this changelog check is failing...

Most of the other test fails are fixed by #301

nabobalis · 2026-02-12T06:24:49Z

I do not understand why this changelog check is failing...

I think since there are two and one is not for this PR.

wtbarnes · 2026-02-12T13:39:38Z

I do not understand why this changelog check is failing...

I think since there are two and one is not for this PR.

Ah I didn't realize that other one got modified.

Cadair

This looks really good.

One general thing is that spacing around operators is chaotic in this PR.

docs/how_to_guide/adding_a_coalignment_method.rst

Cadair · 2026-02-18T16:53:24Z

sunkit_image/coalignment/interface.py

+]
+
+
+class AffineParams(NamedTuple):


mostly out of curiosity. Why NamedTuple here rather than say dataclass?

No reason, would you prefer a dataclass?

Moved to a dataclass.

Cadair · 2026-02-18T16:55:25Z

sunkit_image/coalignment/interface.py

+
+    .. note::
+
+        This function is intended to correct maps with inaccurate metadata.


Is it worth explaining here that this only handles affine transform like modification of the metadata?

sunkit_image/coalignment/interface.py

Cadair · 2026-02-18T17:02:42Z

sunkit_image/coalignment/match_template.py

+    # Particularly for this method, there is no change in the rotation or scaling,
+    # hence the hardcoded values of scale to 1.0 & rotation to identity matrix
+    scale = np.array([1.0, 1.0])
+    rotation_matrix = np.eye(2)
+    return AffineParams(scale=scale, rotation_matrix=rotation_matrix, translation=(x_shift, y_shift))


This is a more concrete example of my comment about NamedTuple vs something else. Something else could have defaults of unity for everything? (Is explicit better though? maybe?)

Not fussed.

🤷‍♂️ I don't have a strong opinion here. We could always switch to using a dataclass later on.

Cadair · 2026-02-18T17:05:54Z

docs/code_ref/coalignment.rst

+.. automodapi:: sunkit_image.coalignment.match_template
+
+.. automodapi:: sunkit_image.coalignment.phase_cross_correlation
+
+.. automodapi:: sunkit_image.coalignment.interface
+    :no-inheritance-diagram:
+    :skip: coalign
+
+.. automodapi:: sunkit_image.coalignment.register
+    :no-heading:


These have the wrong heading levels, they should be sub-headings to sunkit_image.coalignment.

I revamped the entire reference section instead

This looks way better to me. Thanks!

Cadair · 2026-02-18T17:06:54Z

sunkit_image/coalignment/register.py

+]
+
+# Global Dictionary to store the registered methods and their names
+REGISTERED_METHODS = {}


This probably isn't thread-safe 😆

How would we make it thread safe?

No idea, but we should probably at least track it. Maybe Albert has some ideas given his work on sunpy.

sunkit_image/coalignment/tests/test_coalignment.py

Cadair · 2026-02-18T17:14:58Z

sunkit_image/coalignment/interface.py

+    # NOTE: Currently, the only metadata updates that are supported are shifts in
+    # the reference coordinate. Once other updates are supported, this check can be removed.
+    if not (affine_params.rotation_matrix == np.eye(2)).all():
+        raise NotImplementedError('Changes to the rotation metadata are currently not supported.')
+    if not (affine_params.scale == np.array([1,1])).all():
+        raise NotImplementedError('Changes to the pixel scale metadata are currently not supported.')


Can we open an issue for this? and document that they are currently unsupported? Also edit these error messages to tell people that they should come ask us about them if they have a use for them.

I added a message to open an issue with a link.

sunkit_image/coalignment/interface.py

Co-authored-by: Stuart Mumford <stuart@cadair.com>

nabobalis · 2026-02-18T19:00:50Z

I handled the rename and updating the api docs.
When we are ok with these changes, I plan to ruff format the PR/repo

wtbarnes · 2026-02-20T06:32:49Z

docs/code_ref/stara.rst

@@ -1 +1,7 @@
+Granule Detection (`sunkit_image.granule`)


This one is mislabeled. It should be STARA.

nabobalis · 2026-02-20T19:43:41Z

Looks like tox update decided to break things.

Deus1704 and others added 30 commits June 12, 2024 17:31

a better approach to dev

bb4876a

added decorator and incorporated suggestions

08a0ff9

Merge branch 'main' into new_coalignment_api

e02eb28

updated the docstring

f25076b

added tests for majority of all funcs

98c384e

adds how to guide

47e1256

few fixes in refs fixing the refs

temporary de-referencing funcs and methods

73373fc

fix doc tree

125e78a

added refs again

53fd4d5

importing funcs testing import i think final final nail explicit imports

Apply suggestions from code review

4150115

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

code review and fixing minor mistakes

0ac31ab

changed doc to ref

ffe357b

Update ref

adc8bcb

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

better linking

bc6d2e1

lint

Merge remote-tracking branch 'upstream/main' into new_coalignment_api

31d62fa

updated the api, now returns the map with updated metadata

5d67806

updated the how_to_guide

89f6d99

docstring fix

963e476

first try at replacing old coalign

ba8ec82

removing old coalignment completely, refs fixed, old example removed

fcf9eae

fix the issue of method not being registered

7dba720

fix the pixel pair issue

93a477b

Apply suggestions from code review & testing bot

177173d

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

Merge remote-tracking branch 'upstream/main' into new_coalignment_api

466f8e8

fix the order of the reference and target maps

b7ac50c

calm down pre-commit

573d1e3

Merge remote-tracking branch 'upstream/main' into new_coalignment_api

0cee8f1

fixed the method & added examples and tests

81b7fcb

adding the eis file

587833b

Apply suggestions from code review

9d6ab7f

Co-authored-by: Nabil Freij <nabil.freij@gmail.com>

nabobalis force-pushed the coalign_pr branch from 240258d to 01c1344 Compare November 19, 2025 23:23

Fixed scale check

2917d48

wtbarnes mentioned this pull request Jan 9, 2026

New Coalignment API #207

Closed

7 tasks

wtbarnes added 7 commits February 11, 2026 17:47

Merge branch 'main' into coalign_pr

345956a

correct changelog PR

6498533

extra figure and minor edits to gallery example

1e79fc8

minor reorganizaiton of coalignment API docs

92352d0

reorganize how-to guide

10ec3f4

docstring cleanup

da37e04

tighten up test tolerance

1d9a6ef

nabobalis requested a review from wtbarnes February 12, 2026 06:24

Merge branch 'main' into coalign_pr

e6114eb

Cadair added the No Changelog Entry Needed Skip all changelog checks. label Feb 18, 2026

Cadair reviewed Feb 18, 2026

View reviewed changes

sunkit_image/coalignment/interface.py Outdated Show resolved Hide resolved

Cadair reviewed Feb 18, 2026

View reviewed changes

sunkit_image/coalignment/interface.py Outdated Show resolved Hide resolved

nabobalis and others added 2 commits February 18, 2026 09:43

Apply suggestions from code review

c525b62

Co-authored-by: Stuart Mumford <stuart@cadair.com>

[Fix] Emdashes are important for writing english -- vital even

7d0adc1

nabobalis mentioned this pull request Feb 18, 2026

Coalignment does not handle scale or rotation. #303

Open

nabobalis force-pushed the coalign_pr branch from a1fef30 to 15292d7 Compare February 18, 2026 19:12

Update figure env

8e45e74

nabobalis force-pushed the coalign_pr branch from baa86ea to 8e45e74 Compare February 18, 2026 19:28

Fix formatting

b731031

wtbarnes approved these changes Feb 20, 2026

View reviewed changes

Rename section to Sunspot Tracking And Recognition

c1bb514


		.. note::

		This function is intended to correct maps with inaccurate metadata.

		]


		class AffineParams(NamedTuple):

Uh oh!

Comments

Conversation

nabobalis commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtbarnes commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nabobalis commented Feb 12, 2026

Uh oh!

wtbarnes commented Feb 12, 2026

Uh oh!

Cadair left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nabobalis commented Feb 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nabobalis commented Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nabobalis commented Nov 18, 2025 •

edited

Loading

wtbarnes commented Feb 12, 2026 •

edited

Loading