Alma deprecation cleanup #2309
Alma deprecation cleanup #2309
Conversation
|
(Remote tests run forever, I still have to double-check whether this doesn't introduce any more of the to fail, so hold back from merging) |
Codecov Report
@@ Coverage Diff @@
## main #2309 +/- ##
==========================================
+ Coverage 62.97% 63.07% +0.10%
==========================================
Files 131 131
Lines 17073 17031 -42
==========================================
- Hits 10752 10743 -9
+ Misses 6321 6288 -33
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
|
Apparently, the remote tests are full of |
bsipocz
force-pushed
the
alma_deprecation_cleanup
branch
from
a3abb28
to
40868ff
Compare
| @@ -372,11 +272,11 @@ def test_doc_example(self, temp_dir, alma): | |||
|
|
|||
| assert X30.sum() == 4 # Jul 13, 2020 | |||
| assert X31.sum() == 4 # Jul 13, 2020 | |||
| mous1 = alma.stage_data('uid://A001/X11f/X30') | |||
| mous1 = alma.get_data_info('uid://A001/X11f/X30') | |||
There was a problem hiding this comment.
now that the narrative documentation is tested, I think this whole test function can be removed. What do you think @keflavich?
There was a problem hiding this comment.
the stage_data function? yes.
There was a problem hiding this comment.
I've commented on an awkward line, I meant to remove the whole test_doc_example function as hopefully of that is already covered in the narrative docs itself? (It's not verbatim as I see, e.g. the sizes are not checked, but if you think that part of the test is useful this test function may need to be renamed and scoped down?)
There was a problem hiding this comment.
oh... I don't know. If the tests are working, is there any harm in having them duplicated in the docs and the tests? But if it really is a duplication I have no objection to removing the copy in the test file.
There was a problem hiding this comment.
The alma tests are already among the slowest, so the only "harm" in running them multiple times is that some of the remote data tests run long, and it all adds up (this one is one of the slowest).
Anyway, it points beyond this PR so I leave this one alone for now.
| @@ -619,7 +519,7 @@ def test_staging_postfeb2020(alma): | |||
| @pytest.mark.remote_data | |||
| @pytest.mark.skip('Not working for now - Investigating') | |||
| def test_staging_uptofeb2020(alma): | |||
| tbl = alma.stage_data('uid://A001/X121/X4ba') | |||
There was a problem hiding this comment.
do we still need this test function and the previous, test_staging_postfeb2020? 2 year have passed since that API change, I suspect these can now be removed.
|
Remote tests all pass, so I'm going ahead and merge this to get this into the current release. |
I was run into these very old deprecations from prior using pyvo while investigating an upstream bug report, and couldn't resist to do a bit of cleanup.
@andamian - are you OK with the change of making kwargs kwarg only? I needed that for the methods where some of the deprecated ones got removed, and then had a bold thought and did it for all methods.