Update orientation API to match UI #3128
Conversation
|
Added deprecation warnings, note that I left a few things that are only used internally that reference link type (e.g., LinkUpdateMessage). If that seems like it could be confusing I could rename those things as well. |
rosteen
requested review from
javerbukh,
pllim,
kecnry,
bmorris3,
haticekaratay,
cshanahan1 and
gibsongreen
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3128 +/- ##
==========================================
- Coverage 88.85% 88.81% -0.04%
==========================================
Files 112 112
Lines 17409 17430 +21
==========================================
+ Hits 15468 15481 +13
- Misses 1941 1949 +8 ☔ View full report in Codecov by Sentry. |
jdaviz/configs/imviz/helper.py
Outdated
|
|
||
| def get_link_type(self, data_label_1, data_label_2): | ||
| logging.warning("DeprecationWarning: get_link_type has been replaced by " |
There was a problem hiding this comment.
Why logging.warning instead of normal warnings.warn? Or you can also use the @deprecated decorator from astropy.utils?
This comment applies to all other similar occurrences.
There was a problem hiding this comment.
You know, I was just being lazy and copied what the first DeprecationWarning I greped in the repo was doing. I'll update it to use the astropy decorator.
There was a problem hiding this comment.
Are you sure it is updated? I still see logging.warning in the diff instead of @deprecated.
There was a problem hiding this comment.
Yes, I see the astropy deprecation in the diff. Refresh?
There was a problem hiding this comment.
I refreshed, but no change. You sure you pushed that commit out?
There was a problem hiding this comment.
Ahh I wasn't paying attention to where this comment was, thought you were talking about the other file and forgot there was one here. One sec...
Good call, I'll update those as well. |
|
CI refused to run, maybe due to conflict. Please rebase. Thanks! |
Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
rosteen
force-pushed
the
update-orientation-api
branch
from
1f3011e to
e3867e1
Compare
|
|
||
|
|
||
| @pytest.mark.filterwarnings(r"ignore:The .* function is deprecated") |
There was a problem hiding this comment.
@kecnry , is there a more elegant way to exclude deprecated traitlets from such a test rather than a blanket ignore?
There was a problem hiding this comment.
the other option would be an if-statement in the for-loop (if attr in (...): continue). That takes more work to keep updated since individual entries need to be added manually, but also could act as a list of things to remind us to remove them when the deprecation period is over.
There was a problem hiding this comment.
Hopefully deprecating stuff isn't something we're going to do every day, so should we go the more explicit route?
There was a problem hiding this comment.
I made this explicit, there were only two so it wasn't too much.
Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
Changes
link_typetoalign_byandwcs_use_affinetofast_approximationto match the text that is in the UI. Opening as draft since I still need to add deprecated versions of the old API.