-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Remove things deprecated since v3.9 #2878
Conversation
I think there are JIRA tickets for these but I cannot find them. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2878 +/- ##
==========================================
- Coverage 88.72% 88.68% -0.05%
==========================================
Files 111 109 -2
Lines 17102 16993 -109
==========================================
- Hits 15174 15070 -104
+ Misses 1928 1923 -5 ☔ View full report in Codecov by Sentry. |
docs/imviz/plugins.rst
Outdated
.. _rotate-canvas: | ||
|
||
Canvas Rotation | ||
=============== | ||
|
||
.. note:: | ||
|
||
This plugin is deprecated in favor of rotation via :ref:`imviz-orientation` and will be removed | ||
in a future release. | ||
|
||
The canvas rotation plugin allows rotating and horizontally flipping the image to any arbitrary | ||
value by rotating the canvas axes themselves. Note that this does not affect the underlying data, and | ||
exporting data to the notebook via the API will therefore not exhibit the same rotation. | ||
|
||
The :ref:`imviz-compass` will also rotate (and flip) accordingly, but will show the zoom box | ||
corresponding to the zoom limits, not the region shown in the viewer itself. | ||
|
||
Presets are provided to reset the orientation as well as to set north up and east either to the | ||
right or the left, as well as a slider and input to set the angle and a switch to set whether the | ||
axes should be flipped horizontally after applying the rotation (a vertical flip can be achieved | ||
via a 180 deg rotation and a horizontal flip). | ||
|
||
Due to browser limitations, Canvas Rotation is only available on Chromium-based browsers. | ||
This plugin was removed in Jdaviz v4.0, use :ref:`imviz-orientation` instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when (if ever) can we remove this section entirely (or maybe hide it somewhere but keep the link from breaking)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. _rotate-canvas:
needs a section to work, but it doesn't have to be on this page. And we can always break the doc anchor link in the future, or even in this PR if you don't think anyone is using it or it doesn't matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can create a page dedicated to removed/deprecated stuff and migrate everything there? I don't suspect this was widely used, so it would be nice to clean it up and remove it from the main plugin page, but we can also revisit that later down the road if you think it should remain here for a little while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK for simplicity, I just completely removed the section in this PR. If anyone complains about broken link, they can send in help call but like you said, probably no one. Can we merge now? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, already approved
Description
This pull request is to remove API and plugin that were deprecated since v3.9.
Fix #2672
Change log entry
CHANGES.rst
? If you want to avoid merge conflicts,list the proposed change log here for review and add to
CHANGES.rst
before merge. If no, maintainershould add a
no-changelog-entry-needed
label.Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
trivial
label.