depr tests #372
depr tests #372
Conversation
sbillinge
commented
Jan 17, 2026
sbillinge
commented
- deprecated: loadData to load_data, and move it
- fix: bad imports so tests pass
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.7.0 #372 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 9 9
Lines 516 547 +31
=========================================
+ Hits 516 547 +31
🚀 New features to boost your workflow:
|
|
@sbillinge one comment inline |
I couldn't see an inline comment, did you maybe forget to save it (that happens to me sometimes)? |
| * All data blocks findable by load_data. | ||
| * Headers (if present) for each data block. (Generally the headers | ||
| contain column name information). | ||
| """ |
There was a problem hiding this comment.
Do we want to replace the code of the deprecated function so it points to the new function? that way we don't have duplicate code. So it would look something like:
@ deprecated(loaddata_deprecation_msg)
def loadData(inputs):
'''leave the docstring here unchanged so mouse-over shows it properly```
from diffpy.utils.tools import load_data
return load_data(inputs)
This is a unique case because youve moved this function to tools.py, but in most cases it would be something like return self.new_func_name()
There was a problem hiding this comment.
Yah, that's a good point. The docstrings will be wrong for the old function, but we kind of want to point people to the new one anyway.
Do you know what happens to the API docs with the deprecated function? We kind of don't want it showing up in the API docs. I just thought of that too. Your way is probably better in this regard too as there is no explicit discussing in the old function any more.
There was a problem hiding this comment.
Yah, that's a good point. The docstrings will be wrong for the old function, but we kind of want to point people to the new one anyway.
Do you know what happens to the API docs with the deprecated function? We kind of don't want it showing up in the API docs. I just thought of that too. Your way is probably better in this regard too as there is no explicit discussing in the old function any more.
There was a problem hiding this comment.
@sbillinge thats a good point about api docs. In that case I think we should probably replace the docstring in the old function with something like "This function is deprecated. Please use diffpy.utils.wherever.new_function." That way, if someone tries to mouse-over it, they can see what the new function is. I think no matter what we will have duplicates in the api docs temporarily, but the docstring will point to the new function so no big deal. I don't know how the api docs are built from the docstring in the deprecated function case however we can easily check.
There was a problem hiding this comment.
yes yes yes! Nice. OK, we are making progress here. I like this approach. I wonder if we want to quickly catch the steps and write a little chapter in the docs of scikit-package (which is our kind of de-facto group standards for doing everything)? The advantage is that we can then share around these deprecation tasks. I am about to make anaother push.
In other news, I have changed the base branch of this PR to a branch called v3.7.0. Let's make all our deprecation work into that. I can then make a patch-release from main when we get our free minutes back from GH and we can start this work in earnest starting on Feb 1!
There was a problem hiding this comment.
@sbillinge I did this here a while ago. It might need a little update based on this convo but the bones are good: #366 (comment)
|
@sbillinge yep sorry, forgot to submit! should be visible now |
2 participants
