-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update syntax for installing from git repos to latest recommended #12
Conversation
…ich fixes the sunpy build
@pllim - I think it does make sense to run in PRs, so I think we should permanently add that setting. |
I think the test failures might be related to the fact that there are some pytest options specified in sunpy's setup.cfg, but we'd have to repeat these here as command-line arguments. I wonder if there's a way to avoid that though as that would be a pain to keep in sync. Will ping the sunpy folk. For example some of the test failures are probably due to the fact that we require the equivalent of:
|
@@ -2,6 +2,7 @@ name: astropy_rc_basic | |||
|
|||
on: | |||
workflow_dispatch: | |||
pull_request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is no security concern, I guess it is okay to leave this in? This is a low-traffic repo anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any potential security concerns with this. And any PR is likely to change the config in a way we want to make sure it isn't breaking I think.
astroquery,all: git+https://github.com/astropy/astroquery.git#egg=astroquery[test,all] | ||
asdf_astropy,all: asdf_astropy[test] @ git+https://github.com/astropy/asdf-astropy.git | ||
astropy_healpix,all: astropy_healpix[test] @ git+https://github.com/astropy/astropy-healpix.git | ||
astroquery,all: astroquery[test,all] @ git+https://github.com/astropy/astroquery.git | ||
ccdproc,all: psutil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops did I use the wrong syntax to pull in dependencies? My bad!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this original syntax did work at some point but I think it's not the latest and maybe doesn't work with new features like what sunpy uses. In any case, this should now be the preferred syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The diff looks good. Should we investigate the actual sunpy failure separately?
Yes sounds good |
Fixes #11 (the sunpy build)
There are however now genuine test failures in the sunpy build