getAvailability changes/deprecation warning message#33
getAvailability changes/deprecation warning message#33lkeyson-earthscope wants to merge 11 commits into
Conversation
|
What is the issue with ObsPy versions? |
|
When running on an older version of python, it prompted me to update obspy -- but then the version of python I was on was old enough that it would fail to update, and then would run the metrics code after that. Because it was failing, every time I ran the code, it prompted me and then wen through the entire try-to-update-but-fail process before running metrics each time. If I decided to not update obspy, it would just quit out of ISPAQ. Shouldn't be a problem for a newer install like this, but it's possible that in the future the same thing could happen again and I'd rather it give a warning about it than force require an update. |
|
Output when running ISPAQ now looks something like this (when running PH5) Where we have the ISPAQ messaging at top, and the Rcode messaging at the end |
| ... | ||
| """ | ||
| logging.getLogger(__name__).warning( | ||
| "getAvailability will be deprecated in a future release. " |
There was a problem hiding this comment.
This should say it is deprecated now and not in a future release. It will be defunct and removed in a future release.
There was a problem hiding this comment.
Suggest:
"getAvailability is deprecated and will be removed in a future release. The EarthScope fdsnws/station service no longer supports 'matchtimeseries' and 'includeavailability' parameters which are used by getAvailability. Without those parameters, getAvailability currently returns the same results as the getChannel function."
There was a problem hiding this comment.
Hm, there is a difference between the getAvailability in ISPAQ and that of IRISSeismic. This one can be deprecated now or could continue to exist while the underlying IRISSeismic::getAvailability has been changed (but not removed yet). The message does currently indicate that the upstream IRISSeismic::getAvailability has been changed to remove the parameters and will return the same as getChannel, but in reality the ISPAQ getAvailability function hasn't changed at all. matchtimeseries and includeavailability aren't even fields that can be passed along in ISPAQ's getAvailability.
I could at this point move ISPAQ off of ISPAQ's getAvailability and deprecate the function, that was my original plan and it's one of the changes I made before reverting back to using IRISSeismic::getAvailability.
There was a problem hiding this comment.
matchtimeseries and includeavailability are hardcoded as parameters inside IRISSeismic getAvailability, but skipped if the source is PH5. So it does have this dependency, but it still works because of how ISPAQ is using it (or not using it). We're not intending ISPAQ to be used as a Python library for the CRAN R-code by users, so you're right that it doesn't really matter if it's explicitly deprecated or not. I think the important thing is to replace our use of the function so that when it gets removed from the R-code, ISPAQ does not break.
There was a problem hiding this comment.
Oh, I thought that the updated v1.9.0 IRISSeismic::getAvailability removed those hardcoded matchtimeseries and includeavailability so that IRISSeismic::getAvailability no longer included those parameters at all. And then on the ISPAQ side, our only call out of (ISPAQ's) getAvailability includes the following:
_R_getAvailability(
r_client,
network,
station,
location,
channel,
starttime,
endtime,
includerestricted,
latitude,
longitude,
minradius,
maxradius,
)
And then, yeah all other calls to the station service are done using obspy, which defaults to none for both matchtimeseries and includeavailability -- and ISPAQ doesn't provide either of those parameters for those calls either.
Regardless of the IRISSeismic::getAvailability behavior, I've switch back to having ISPAQ to use ISPAQ's getChannel, which relies on IRISSeismic::getChannel. So we're off of that dependency.
There was a problem hiding this comment.
You're right, IRISSeismic did remove the parameters. I need more coffee.
There was a problem hiding this comment.
Ha, no problem - I think we're all on the same page now and I'm no longer second guessing my understanding of the changes to mustang-metrics
|
This line 1025 of ispaq/concierge.py should be updated to use getChannel:
Even though we're keeping the function available in ISPAQ, we shouldn't actually use the deprecated function when there is an alternative that returns the same result. We can just fix it and it will be transparent to the user with no warnings that they can't take action on. |
|
The CHANGELOG.txt file needs to be updated. I think this is the best place to communicate the code changes rather than having warnings show up when running run_ispaq.py. |
I had originally made this change and then reverted to use getAvailability again since the underlying IRISSeismic::getAvailability still exists and works. But I can change it back to use ISPAQ's getChannel. |
The IRISSeismic package has deprecatde getAvailability, so this MR moves ISPAQ off of any dependency that it has on that function.