Skip to content

fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models - #39830

Merged
qubvel merged 6 commits into
huggingface:mainfrom
sbucaille:rework-keypoint-matching-visualization
Aug 1, 2025
Merged

fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models#39830
qubvel merged 6 commits into
huggingface:mainfrom
sbucaille:rework-keypoint-matching-visualization

Conversation

@sbucaille

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds visualize_keypoint_matching to LightGlue and SuperGlue image processor
Deprecate plot_keypoint_matching from LightGlue image processor

Who can review?

@qubvel @stevhliu

@stevhliu stevhliu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, docs LGTM!

@qubvel qubvel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sbucaille Thanks for making it standardized! Just a few comments


return results

def visualize_keypoint_matching(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we use # Copied from?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in 1a45ea3

) -> list[dict[str, torch.Tensor]]:
return super().post_process_keypoint_matching(outputs, target_sizes, threshold)

def visualize_keypoint_matching(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in 1a45ea3

plt.scatter(keypoint1_x + width0, keypoint1_y, c="black", s=2)
plt.show()

def _get_color(self, score):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in 1a45ea3


return results

def visualize_keypoint_matching(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in 1a45ea3

Comment on lines +465 to +475
def plot_keypoint_matching(self, images: ImageInput, keypoint_matching_output):
"""
Plots the image pairs side by side with the detected keypoints as well as the matching between them. Requires
matplotlib to be installed.

.. deprecated::
`plot_keypoint_matching` is deprecated and will be removed in a future version. Use `visualize_keypoint_matching` instead.

Args:
images (`ImageInput`):
Image pairs to plot. Same as `SuperGlueImageProcessor.preprocess`. Expects either a list of 2 images or

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no need to add this method, it's going to be removed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I went autopilot there, didn't mean to 😅

@github-actions

github-actions Bot commented Aug 1, 2025

Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: efficientloftr, lightglue, superglue

@qubvel qubvel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, just a nit and ready to be merged!

- preprocess
- post_process_keypoint_matching
- plot_keypoint_matching
- visualize_keypoint_matching

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's add the same for superglue

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Indeed, forgot about this one, fixed !

@qubvel
qubvel enabled auto-merge (squash) August 1, 2025 16:18
@qubvel
qubvel merged commit 1ec0fec into huggingface:main Aug 1, 2025
16 checks passed
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@sbucaille
sbucaille deleted the rework-keypoint-matching-visualization branch August 1, 2025 16:33
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
…e_keypoint_matching` as a standard (huggingface#39830)

* fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models

* refactor: added copied from

* fix: make style

* fix: repo consistency

* fix: make style

* docs: added missing method in SuperGlue docs
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
…e_keypoint_matching` as a standard (huggingface#39830)

* fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models

* refactor: added copied from

* fix: make style

* fix: repo consistency

* fix: make style

* docs: added missing method in SuperGlue docs
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
…e_keypoint_matching` as a standard (huggingface#39830)

* fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models

* refactor: added copied from

* fix: make style

* fix: repo consistency

* fix: make style

* docs: added missing method in SuperGlue docs
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
…e_keypoint_matching` as a standard (huggingface#39830)

* fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models

* refactor: added copied from

* fix: make style

* fix: repo consistency

* fix: make style

* docs: added missing method in SuperGlue docs
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
…e_keypoint_matching` as a standard (huggingface#39830)

* fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models

* refactor: added copied from

* fix: make style

* fix: repo consistency

* fix: make style

* docs: added missing method in SuperGlue docs
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
…e_keypoint_matching` as a standard (huggingface#39830)

* fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models

* refactor: added copied from

* fix: make style

* fix: repo consistency

* fix: make style

* docs: added missing method in SuperGlue docs
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
…e_keypoint_matching` as a standard (huggingface#39830)

* fix: deprecate plot_keypoint_matching and make visualize_keypoint_matching for all Keypoint Matching models

* refactor: added copied from

* fix: make style

* fix: repo consistency

* fix: make style

* docs: added missing method in SuperGlue docs
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.

4 participants