Skip to content

Deprecate astroquery/utils/download_file_list.py#2247

Merged
bsipocz merged 2 commits into
astropy:mainfrom
eerovaher:rm-download-file-list
Dec 17, 2021
Merged

Deprecate astroquery/utils/download_file_list.py#2247
bsipocz merged 2 commits into
astropy:mainfrom
eerovaher:rm-download-file-list

Conversation

@eerovaher

@eerovaher eerovaher commented Dec 13, 2021

Copy link
Copy Markdown
Member

The contents of this file were not being used anywhere, so it should be safe to remove even without deprecating first.

EDIT: This pull request will deprecate the module, it will be removed later.

@codecov

codecov Bot commented Dec 13, 2021

Copy link
Copy Markdown

Codecov Report

Merging #2247 (a96365f) into main (8a60cd2) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2247      +/-   ##
==========================================
+ Coverage   62.38%   62.40%   +0.02%     
==========================================
  Files         131      131              
  Lines       16775    16777       +2     
==========================================
+ Hits        10465    10470       +5     
+ Misses       6310     6307       -3     
Impacted Files Coverage Δ
astroquery/utils/download_file_list.py 28.12% <100.00%> (+7.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a60cd2...a96365f. Read the comment docs.

@keflavich

Copy link
Copy Markdown
Contributor

This is a public function, so technically someone could be using it standalone. However, I don't think it's very likely since we didn't document it well.

I'm ambivalent about removing this: since we don't use it, it's fine to remove, but it looks like something that could be useful. Within astroquery, though, we should stick to the other extensive download tools we've built.

@bsipocz

bsipocz commented Dec 13, 2021

Copy link
Copy Markdown
Member

I'm cutting 0.4.5 this week, let's have this milestone for 4.0, we can merge this when we know for sure that it's the next one coming (I wish it was, but I cannot say it for sure as we need to do a lot of cleanup work for it).

In the meantime I'm not opposed to have this merged whenever, of you decorate it as deprecation.

@eerovaher

Copy link
Copy Markdown
Member Author

Looks like deprecation is preferred over immediate removal. The module only provides one public function (__all__ = ['download_list_of_fitsfiles']), so I will deprecate that.

This commit deprecates the function `download_list_of_fitsfiles()`, and
because that is the only public function defined by the aforementioned
file then the whole file becomes deprecated.
@eerovaher eerovaher force-pushed the rm-download-file-list branch from 2d84be1 to a96365f Compare December 14, 2021 13:26
@eerovaher eerovaher changed the title Remove astroquery/utils/download_file_list.py Deprecate astroquery/utils/download_file_list.py Dec 14, 2021
@bsipocz bsipocz added the utils label Dec 17, 2021
@bsipocz bsipocz added this to the v0.4.5 milestone Dec 17, 2021
@bsipocz

bsipocz commented Dec 17, 2021

Copy link
Copy Markdown
Member

Thanks, this is now uncontroversial.

@bsipocz bsipocz merged commit 17c79f9 into astropy:main Dec 17, 2021
@eerovaher eerovaher deleted the rm-download-file-list branch December 27, 2021 14:23
syed-gilani pushed a commit to syed-gilani/astroquery that referenced this pull request Mar 11, 2022
burnout87 pushed a commit to oda-hub/astroquery that referenced this pull request May 10, 2022
mhsarmiento pushed a commit to esdc-esac-esa-int/astroquery that referenced this pull request May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants