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.

Sign up for GitHub

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

Jump to bottom

Updating astropy-helpers to latest v2.0dev #6140

Merged
merged 3 commits into from Jun 26, 2017
Merged

Updating astropy-helpers to latest v2.0dev #6140

merged 3 commits into from Jun 26, 2017

Conversation

bsipocz
Copy link
Member

@bsipocz bsipocz commented May 30, 2017 •
edited

DO NOT MERGE! This is a testing, placeholder PR until ah is released.

EDIT: this PR now contains only the helpers update to 2.0rc1 and related docs changes. Can be merged once CI passes.

There were quite a few changes in the helpers, and thus I see only benefits of trying to test against the latest dev branch well before the feature freeze.

E.g. I expect a few docs issues with the new sphinx version, but we need this helpers to be able to test it.

@bsipocz bsipocz added this to the v2.0.0 milestone May 30, 2017
@bsipocz bsipocz force-pushed the astropy_helpers_v2.0dev branch 2 times, most recently from 09ebe26 to 4aa15db Compare May 31, 2017 14:04
@bsipocz bsipocz mentioned this pull request May 31, 2017
Closed
@bsipocz bsipocz force-pushed the astropy_helpers_v2.0dev branch 2 times, most recently from 2dab050 to deef1c3 Compare June 1, 2017 13:53
@bsipocz
Copy link
Member Author

bsipocz commented Jun 3, 2017

I'm not sure where is this undefined reference:
https://travis-ci.org/astropy/astropy/jobs/238946509#L4481

The other warnings seem to be due to #4927 and thus will need to be fixed before the release.

@bsipocz bsipocz added this to High priority in 2.0 Feature Planning Jun 12, 2017
@bsipocz bsipocz force-pushed the astropy_helpers_v2.0dev branch 2 times, most recently from 9b0a13d to fc6876c Compare June 14, 2017 11:52
@pllim pllim mentioned this pull request Jun 15, 2017
Closed
@bsipocz
Copy link
Member Author

bsipocz commented Jun 20, 2017

I think the helpers is now in a working state, but nevertheless, we need to sort out the docs warnings before having the release. Currently there are 11 of them, mostly related to #6232 and #4927.

@pllim
Copy link
Member

pllim commented Jun 20, 2017 •
edited

See sphinx-gallery/sphinx-gallery#255 for potential fix to the RemovedInSphinx17Warning in test log. (Maybe use my feature branch on Travis until that PR is merged?) This fix is not essential for the test passing, so postponed to #6266.

See astropy/astropy-data#26 for potential fix to the "Invalid 'BLANK' keyword" and "[LON/LAT]POLE2" in test log. (The proper way is to fix the data like I did, but if desperate, we now know what files cause these warnings, so can just track them down in the doc and silence the warnings.)

See #6261 for potential fix to the MaskedArrayFutureWarning in test log.

Also see bsipocz#2 for potential fix to "Ripley footnote not referenced" warnings.

This was referenced Jun 21, 2017
Closed
Merged
@pllim
Copy link
Member

pllim commented Jun 21, 2017 •
edited

Update for today: I removed my Ripley ref fix in favor of #6252 and rebased against latest master.

(Sorry if I messed up the commit history...)

@bsipocz
Copy link
Member Author

bsipocz commented Jun 21, 2017

@pllim - Don't worry about the commit history here, at the end of the day all the sphinx fixing commit should go into another PR, and this will only contain the astropy helpers update. But currently it's more convenient to test the former here, too as it needs the fixes from the helpers.

@pllim
Copy link
Member

pllim commented Jun 22, 2017 •
edited

@bsipocz , I hunted down the R115 warning, which is caused by wrong indentation placement at https://github.com/astropy/astropy/blob/master/astropy/modeling/optimizers.py#L187 !

EDIT: For completeness, here is why it failed -- Reference cannot be in the first sentence, because when the module members TOC is generated, it tries to find the link that is in another page. (Something like that.)

This was referenced Jun 22, 2017
Merged
Merged
@pllim pllim mentioned this pull request Jun 22, 2017
Merged
@pllim
Copy link
Member

pllim commented Jun 22, 2017

Update: I understand all the warnings now. Some are fixed here and some need external fixes. See #6140 (comment)

@bsipocz
Copy link
Member Author

bsipocz commented Jun 22, 2017

@pllim - I've restarted the build now that the data file updates are merged.

@pllim
Copy link
Member

pllim commented Jun 22, 2017

OMG it's green -- https://travis-ci.org/astropy/astropy/jobs/245655085

@pllim
Copy link
Member

pllim commented Jun 22, 2017

@bsipocz , you said the warning fixes should go in a different PR, but how? Are you going to cherry pick, or am I going to copy-and-paste the code into a new branch?

pllim
pllim reviewed Jun 22, 2017
View reviewed changes
.travis.yml Outdated
@@ -144,6 +145,10 @@ install:
- git clone git://github.com/astropy/ci-helpers.git
- source ci-helpers/travis/setup_conda.sh

# Use this until https://github.com/sphinx-gallery/sphinx-gallery/pull/255
# is merged.
- if [[ $SETUP_CMD == *build_docs* ]]; then pip -q install git+https://github.com/pllim/sphinx-gallery.git@stat-iter-deprec#egg=sphinx-gallery; fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This fixes the deprecation warning from Sphinx for sure. This hack will have to stay until my PR over at sphinx-gallery gets merged, which I have no control over. But feel free to suggest a more elegant hack.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this warning cause a sphinx failure? if it does, could we maybe use the functionality of #6223 to turn it off so not just travis, but local builds would pass without issues?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I am not sure. The test log reported failure and the total number of warnings but did not say which ones counted. Should I undo this and see what happens?

@bsipocz
Copy link
Member Author

bsipocz commented Jun 22, 2017

@pllim - I planned to cherry-pick them, but let's have the helpers RC out first, so we don't just do a blind merge, but test it before that.

@pllim
Copy link
Member

pllim commented Jun 22, 2017

I planned to cherry-pick them

Okay, I'll leave you to your magic then. 😄

@pllim pllim mentioned this pull request Jun 22, 2017
Closed
@bsipocz bsipocz force-pushed the astropy_helpers_v2.0dev branch 2 times, most recently from bc4782e to 2ffd31b Compare June 24, 2017 00:32
@bsipocz bsipocz mentioned this pull request Jun 24, 2017
Merged
@bsipocz
Copy link
Member Author

bsipocz commented Jun 24, 2017

OK, so now I've removed all the sphinx fixes from here to #6272, and this PR goes back to test the helpers RC only.

@bsipocz bsipocz changed the title WIP: Updating astropy-helpers to latest v2.0dev Updating astropy-helpers to latest v2.0dev Jun 24, 2017
@bsipocz bsipocz added 💤 merge-when-ci-passes Do not use: We have auto-merge option now. and removed Work in progress labels Jun 24, 2017
@bsipocz
Copy link
Member Author

bsipocz commented Jun 26, 2017

Merging this now with the helpers 2.0rc1, but we need to update to 2.0 before the release.

@bsipocz bsipocz merged commit 2933ac0 into astropy:master Jun 26, 2017
@astrobot
Copy link

@bsipocz - thanks for merging this! However, I noticed the following issue with this pull request:

  • Changelog entry not present (or pull request number missing) and neither the Affects-dev nor the no-changelog-entry-needed label are set

Would it be possible to fix this? Thanks!

If you believe the above to be incorrect (which I - @astrobot - very much doubt) you can ping @astrofrog

@bsipocz bsipocz moved this from High priority to Done in 2.0 Feature Planning Jun 26, 2017
@bsipocz bsipocz mentioned this pull request Jun 26, 2017
Closed
eteq pushed a commit that referenced this pull request Jun 27, 2017
@bsipocz @eteq
Merge pull request #6140 from bsipocz/astropy_helpers_v2.0dev
9b52117 
Updating astropy-helpers to latest v2.0dev
@pllim pllim mentioned this pull request Jul 3, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pllim pllim pllim left review comments

Assignees
No one assigned
Labels
Experimental no-changelog-entry-needed 💤 merge-when-ci-passes Do not use: We have auto-merge option now.
Projects
No open projects
2.0 Feature Planning
Done
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bsipocz @pllim @astrobot