Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAINT: remove choice of 'astropy' #72

Open
bsipocz opened this issue Aug 23, 2023 · 10 comments
Open

MAINT: remove choice of 'astropy' #72

bsipocz opened this issue Aug 23, 2023 · 10 comments

Comments

@bsipocz
Copy link
Member

bsipocz commented Aug 23, 2023

This day and age I think there isn't more reasons to keep the 'astropy' choice for remote-data. This would generize this plugin further.

(OTOH, I can see that a 'custom' choice could work where a domain could be specified, be that specific to astropy or scipy or whatever else)

@pllim
Copy link
Member

pllim commented Aug 23, 2023

I agree that "custom" would make it much more useful, even for affiliated if nothing else. But would that be too complicated to implement? I imagine you might want a combo at some point. Do you have a CLI API in mind?

@bsipocz
Copy link
Member Author

bsipocz commented Aug 23, 2023

I'll separate the 'custom' out to another issue, I don't yet have a working API design in mind for it, and not sure have a capacity to implement, unless it becomes a blocker for other packages to use this.

@pllim
Copy link
Member

pllim commented Aug 23, 2023

Well, astropy does use that option in CI currently:

https://github.com/astropy/astropy/blob/46df1bbb31d61fe1d1895f6a05abac5e3c35d39c/.github/workflows/ci_workflows.yml#L81

So it is bad if this option is removed without an alternative.

@bsipocz
Copy link
Member Author

bsipocz commented Aug 23, 2023

But do you really need to use that as opposed to any or github? (It used to be used a lot more with the visualization stuff on circleCI, but that's all gone, etc.)

@pllim
Copy link
Member

pllim commented Aug 23, 2023

do you really need to use that

Not sure. I (or someone) need to investigate but not today.

Worst case scenario, astropy would have to pin to older pytest-remotedata, which will be weird since they are both in the same org and supposed to be compatible.

@pllim
Copy link
Member

pllim commented Aug 23, 2023

Ah, I think sites.json is still there and that is used a lot in astropy.coordinates tests.

@bsipocz
Copy link
Member Author

bsipocz commented Aug 23, 2023

sites.json is available, in a very stable way on github, so no need for the ST mirror or other workarounds that went into astropy. Or it can all be tested of course with any as well.

@pllim
Copy link
Member

pllim commented Aug 23, 2023

There could be several ways to solve this, yes, but all of them would require code changes somewhere and needs to go through the PR period and so on.

@bsipocz
Copy link
Member Author

bsipocz commented Aug 23, 2023

I certainly suggest a deprecation cycle for this, with e.g. falling back on any or github, not an outright removal.

@pllim
Copy link
Member

pllim commented Aug 24, 2023

I added this to infrastructure tag-up agenda for 2023-08-29 (though I am not sure if we will have quorum due to European vacations and such).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants