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

Misc cleanup #462

Merged
merged 37 commits into from Feb 23, 2016

Conversation

Projects
None yet
3 participants
@cdeil
Member

cdeil commented Feb 19, 2016

Work in progress ... do not merge!

This PR does a lot of cleanup for the upcoming Gammapy 0.4 release:

  • Move setuptools entry-point list from setup.py to setup.cfg
  • Improve script names in the examples folder and add README there
  • Remove some old code and files
  • Remove unused imports and auto-format code using PyCharm.
  • Change the few tests that still used unittest to pytest.
  • Fix warnings from the Sphinx build (see #139)
  • ... more misc cleanup ...

@cdeil cdeil self-assigned this Feb 19, 2016

@cdeil cdeil added this to the 0.4 milestone Feb 19, 2016

@cdeil cdeil changed the title from Updated astropy-helpers to v1.1.1 to Cleanup for upcoming Gammapy 0.4 release Feb 19, 2016

@cdeil cdeil added cleanup and removed astropy labels Feb 19, 2016

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 20, 2016

Member

There's two Sphinx builds that fail because of warnings:
https://travis-ci.org/gammapy/gammapy/builds/110373765

I can't figure out which warning actually makes the build fail!???
(some we've had for the past months and they don't make the sphinx build fail).

@bsipocz - Do you have time to have a look?

Member

cdeil commented Feb 20, 2016

There's two Sphinx builds that fail because of warnings:
https://travis-ci.org/gammapy/gammapy/builds/110373765

I can't figure out which warning actually makes the build fail!???
(some we've had for the past months and they don't make the sphinx build fail).

@bsipocz - Do you have time to have a look?

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 20, 2016

Member

@joleroi - FYI: I've removed some old spectrum code from pyfact in ec8da04 .

Feel free to remove more stuff from hspec or pyfact once it's clear that it's no longer useful to keep as an example of functionality that we want to build into Gammapy or to test against, or let me know and I'll remove it in this PR.

Member

cdeil commented Feb 20, 2016

@joleroi - FYI: I've removed some old spectrum code from pyfact in ec8da04 .

Feel free to remove more stuff from hspec or pyfact once it's clear that it's no longer useful to keep as an example of functionality that we want to build into Gammapy or to test against, or let me know and I'll remove it in this PR.

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 20, 2016

Member

@cdeil - I have time now, so will try to fix appveyor and look at this if the latest build is still failing.

Member

bsipocz commented Feb 20, 2016

@cdeil - I have time now, so will try to fix appveyor and look at this if the latest build is still failing.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 20, 2016

Member

Thanks. From my side, this PR is important for 0.4, and appveyor isn't.

I've always had trouble finding the critical warning in the Sphinx log that makes the build fail.
@bsipocz - Do you know a trick to find it quickly? Is there an "exit on first critical warning" option so that the last warning printed is the one I want to eliminate?

Member

cdeil commented Feb 20, 2016

Thanks. From my side, this PR is important for 0.4, and appveyor isn't.

I've always had trouble finding the critical warning in the Sphinx log that makes the build fail.
@bsipocz - Do you know a trick to find it quickly? Is there an "exit on first critical warning" option so that the last warning printed is the one I want to eliminate?

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 20, 2016

Member

Never saw that error before, I'll try to reproduce it with that docstring example that's claimed to be failing in gammapy/stats/poisson.py.

Member

bsipocz commented Feb 20, 2016

Never saw that error before, I'll try to reproduce it with that docstring example that's claimed to be failing in gammapy/stats/poisson.py.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 20, 2016

Member

Never saw that error before

Can you link to the line in the travis log?

Member

cdeil commented Feb 20, 2016

Never saw that error before

Can you link to the line in the travis log?

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 20, 2016

Member

I think it should be this one, but looking at it it's not in the docstrings, so I'm clueless atm https://travis-ci.org/gammapy/gammapy/jobs/110619007#L2145

Member

bsipocz commented Feb 20, 2016

I think it should be this one, but looking at it it's not in the docstrings, so I'm clueless atm https://travis-ci.org/gammapy/gammapy/jobs/110619007#L2145

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 20, 2016

Member

This pull request update astropy-helpers to 1.1.1 ... maybe it's an issue introduced there?
(there are some other builds that generate docs with other astropy versions that don't fail).

Member

cdeil commented Feb 20, 2016

This pull request update astropy-helpers to 1.1.1 ... maybe it's an issue introduced there?
(there are some other builds that generate docs with other astropy versions that don't fail).

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 20, 2016

Member

I couldn't reproduce, though locally I have these RuntimeWarnings (that aren't raised by sphinx, so it didn't picked them up).

/sw/lib/python3.5/site-packages/matplotlib/scale.py:93: RuntimeWarning: invalid value encountered in less_equal
  mask = a <= 0.0
/Users/bsipocz/munka/devel/gammapy/build/lib.macosx-10.10-x86_64-3.5/gammapy/spectrum/powerlaw.py:75: RuntimeWarning: divide by zero encountered in true_divide
  term1 = e / (-g + 1)
/Users/bsipocz/munka/devel/gammapy/build/lib.macosx-10.10-x86_64-3.5/gammapy/spectrum/powerlaw.py:77: RuntimeWarning: invalid value encountered in multiply
  return term1 * term2
/sw/lib/python3.5/site-packages/numpy/ma/core.py:6365: RuntimeWarning: overflow encountered in power
  result = np.where(m, fa, umath.power(fa, fb)).view(basetype)
<string>:44: RuntimeWarning: divide by zero encountered in true_divide
<string>:44: RuntimeWarning: divide by zero encountered in log10
<string>:44: RuntimeWarning: invalid value encountered in log10
Member

bsipocz commented Feb 20, 2016

I couldn't reproduce, though locally I have these RuntimeWarnings (that aren't raised by sphinx, so it didn't picked them up).

/sw/lib/python3.5/site-packages/matplotlib/scale.py:93: RuntimeWarning: invalid value encountered in less_equal
  mask = a <= 0.0
/Users/bsipocz/munka/devel/gammapy/build/lib.macosx-10.10-x86_64-3.5/gammapy/spectrum/powerlaw.py:75: RuntimeWarning: divide by zero encountered in true_divide
  term1 = e / (-g + 1)
/Users/bsipocz/munka/devel/gammapy/build/lib.macosx-10.10-x86_64-3.5/gammapy/spectrum/powerlaw.py:77: RuntimeWarning: invalid value encountered in multiply
  return term1 * term2
/sw/lib/python3.5/site-packages/numpy/ma/core.py:6365: RuntimeWarning: overflow encountered in power
  result = np.where(m, fa, umath.power(fa, fb)).view(basetype)
<string>:44: RuntimeWarning: divide by zero encountered in true_divide
<string>:44: RuntimeWarning: divide by zero encountered in log10
<string>:44: RuntimeWarning: invalid value encountered in log10
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 21, 2016

Member

I've started to go through and fix the warnings in the docs build that come up when plot directives are executed. Will finish this up tonight and then hopefully the build will also pass on travis-ci.

Member

cdeil commented Feb 21, 2016

I've started to go through and fix the warnings in the docs build that come up when plot directives are executed. Will finish this up tonight and then hopefully the build will also pass on travis-ci.

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 21, 2016

Member

@cdeil - I'm trying to fix things in https://github.com/gammapy/gammapy/compare/master...bsipocz:fixing_docs_mpl_warnings?expand=1, if it gets any success I'll open a PR to here.

Member

bsipocz commented Feb 21, 2016

@cdeil - I'm trying to fix things in https://github.com/gammapy/gammapy/compare/master...bsipocz:fixing_docs_mpl_warnings?expand=1, if it gets any success I'll open a PR to here.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 21, 2016

Member

@bsipocz - Thank you so much!

Member

cdeil commented Feb 21, 2016

@bsipocz - Thank you so much!

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 21, 2016

Member

@cdeil - Should the downloaded examples work as is?

copying downloadable files... [ 12%] /home/travis/build/bsipocz/gammapy/examples/example_plot_background_model.py

copying downloadable files... [ 25%] /home/travis/build/bsipocz/gammapy/examples/wip_bg_cube_model_comparison.py

copying downloadable files... [ 37%] /home/travis/build/bsipocz/gammapy/examples/wip_bg_cube_models_true_reco.py

copying downloadable files... [ 50%] /home/travis/build/bsipocz/gammapy/examples/example_fc_poisson_analytical.py

copying downloadable files... [ 62%] /home/travis/build/bsipocz/gammapy/docs/tutorials/gammapy-spectrum/spectrum_analysis_example.yaml

copying downloadable files... [ 75%] /home/travis/build/bsipocz/gammapy/examples/example_energy_offset_array.py

copying downloadable files... [ 87%] /home/travis/build/bsipocz/gammapy/examples/example_fc_demonstrate_artefact.py

copying downloadable files... [100%] /home/travis/build/bsipocz/gammapy/examples/example_fc_gauss_analytical.py
Member

bsipocz commented Feb 21, 2016

@cdeil - Should the downloaded examples work as is?

copying downloadable files... [ 12%] /home/travis/build/bsipocz/gammapy/examples/example_plot_background_model.py

copying downloadable files... [ 25%] /home/travis/build/bsipocz/gammapy/examples/wip_bg_cube_model_comparison.py

copying downloadable files... [ 37%] /home/travis/build/bsipocz/gammapy/examples/wip_bg_cube_models_true_reco.py

copying downloadable files... [ 50%] /home/travis/build/bsipocz/gammapy/examples/example_fc_poisson_analytical.py

copying downloadable files... [ 62%] /home/travis/build/bsipocz/gammapy/docs/tutorials/gammapy-spectrum/spectrum_analysis_example.yaml

copying downloadable files... [ 75%] /home/travis/build/bsipocz/gammapy/examples/example_energy_offset_array.py

copying downloadable files... [ 87%] /home/travis/build/bsipocz/gammapy/examples/example_fc_demonstrate_artefact.py

copying downloadable files... [100%] /home/travis/build/bsipocz/gammapy/examples/example_fc_gauss_analytical.py
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 21, 2016

Member

Should the downloaded examples work as is?

Some yes, some are broken.

In the coming weeks we'll work a lot on docs, examples, IPython notebooks and also on putting examples and notebooks under continuous testing.

Member

cdeil commented Feb 21, 2016

Should the downloaded examples work as is?

Some yes, some are broken.

In the coming weeks we'll work a lot on docs, examples, IPython notebooks and also on putting examples and notebooks under continuous testing.

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 21, 2016

Member

It seems like that the sphinx brake comes in with astropy-helpers 1.1, and it still passes with 1.0.6.

Not sure yet what went wrong, and how to fix it, but it may be independent of gammapy (although I didn't see this problem with any of the other packages that I upgraded to use 1.1.1).

Member

bsipocz commented Feb 21, 2016

It seems like that the sphinx brake comes in with astropy-helpers 1.1, and it still passes with 1.0.6.

Not sure yet what went wrong, and how to fix it, but it may be independent of gammapy (although I didn't see this problem with any of the other packages that I upgraded to use 1.1.1).

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 21, 2016

Member

It seems like that the sphinx brake comes in with astropy-helpers 1.1, and it still passes with 1.0.6.

Thanks for figuring this out!

Not sure yet what went wrong,

The fail is caused by sphinx-build returning a non-zero exit status.
Isn't there a way to run sphinx-build so that it prints useful info about where / why it failed or make it fail fast, where the issue occurs?

Member

cdeil commented Feb 21, 2016

It seems like that the sphinx brake comes in with astropy-helpers 1.1, and it still passes with 1.0.6.

Thanks for figuring this out!

Not sure yet what went wrong,

The fail is caused by sphinx-build returning a non-zero exit status.
Isn't there a way to run sphinx-build so that it prints useful info about where / why it failed or make it fail fast, where the issue occurs?

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 22, 2016

Member

@cdeil - sadly even if there is a way, I don't know how to do it.

Member

bsipocz commented Feb 22, 2016

@cdeil - sadly even if there is a way, I don't know how to do it.

@joleroi

This comment has been minimized.

Show comment
Hide comment
@joleroi

joleroi Feb 23, 2016

Contributor

@cdeil sorry didn't see this PR before. Remove everything you think has migrated to gammapy classes from pyfact. I do the same.

Is the 0.4 release still planned for the near future. I would think it is important, if we want to get people to try out gammapy.
If you need any help with the cleanup let me know and I will open a new PR (will start with the IRF plots now)

Contributor

joleroi commented Feb 23, 2016

@cdeil sorry didn't see this PR before. Remove everything you think has migrated to gammapy classes from pyfact. I do the same.

Is the 0.4 release still planned for the near future. I would think it is important, if we want to get people to try out gammapy.
If you need any help with the cleanup let me know and I will open a new PR (will start with the IRF plots now)

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 23, 2016

Member

I'll change back to astropy-helpers 1.0.6 for now ... leaving that issue to a future pull request.

Member

cdeil commented Feb 23, 2016

I'll change back to astropy-helpers 1.0.6 for now ... leaving that issue to a future pull request.

@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 23, 2016

Member

@cdeil - Actually you can go with v1.1rc1, the fail comes in rc2.

Member

bsipocz commented Feb 23, 2016

@cdeil - Actually you can go with v1.1rc1, the fail comes in rc2.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 23, 2016

Member

I've split off the astropy-helpers update and issue to #466 .

I'll also revert the change to use matplotlib cycler because it's not available here:
https://travis-ci.org/gammapy/gammapy/jobs/111186186#L936

We can tolerate the matplotlib warnings a while longer and make this change later.

Member

cdeil commented Feb 23, 2016

I've split off the astropy-helpers update and issue to #466 .

I'll also revert the change to use matplotlib cycler because it's not available here:
https://travis-ci.org/gammapy/gammapy/jobs/111186186#L936

We can tolerate the matplotlib warnings a while longer and make this change later.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 23, 2016

Member

Note to self --- the remaining warnings to fix are this one:

$ python docs/tutorials/npred/npred_convolved_significance.py
/Users/deil/code/gammapy/gammapy/stats/poisson.py:242: RuntimeWarning: invalid value encountered in log
  term_b = sqrt(n_observed * log(n_observed / mu_background) - n_observed + mu_background)

And there's this one, which I haven't been able to pinpoint to a given plot directive:

/Users/deil/code/gammapy/build/lib.macosx-10.11-x86_64-3.5/gammapy/spectrum/powerlaw.py:75: RuntimeWarning: divide by zero encountered in true_divide
  term1 = e / (-g + 1)
/Users/deil/code/gammapy/build/lib.macosx-10.11-x86_64-3.5/gammapy/spectrum/powerlaw.py:77: RuntimeWarning: invalid value encountered in multiply
  return term1 * term2
Member

cdeil commented Feb 23, 2016

Note to self --- the remaining warnings to fix are this one:

$ python docs/tutorials/npred/npred_convolved_significance.py
/Users/deil/code/gammapy/gammapy/stats/poisson.py:242: RuntimeWarning: invalid value encountered in log
  term_b = sqrt(n_observed * log(n_observed / mu_background) - n_observed + mu_background)

And there's this one, which I haven't been able to pinpoint to a given plot directive:

/Users/deil/code/gammapy/build/lib.macosx-10.11-x86_64-3.5/gammapy/spectrum/powerlaw.py:75: RuntimeWarning: divide by zero encountered in true_divide
  term1 = e / (-g + 1)
/Users/deil/code/gammapy/build/lib.macosx-10.11-x86_64-3.5/gammapy/spectrum/powerlaw.py:77: RuntimeWarning: invalid value encountered in multiply
  return term1 * term2
@bsipocz

This comment has been minimized.

Show comment
Hide comment
@bsipocz

bsipocz Feb 23, 2016

Member

An alternative fix for the latter one is in commit 184e91d. Not ideal as it doesn't fix the plot itself (I didn't find out which one was that), but a sanity check for zero division in the code.

https://travis-ci.org/bsipocz/gammapy/builds/111220871 checks whether it works on top of this branch.

Member

bsipocz commented Feb 23, 2016

An alternative fix for the latter one is in commit 184e91d. Not ideal as it doesn't fix the plot itself (I didn't find out which one was that), but a sanity check for zero division in the code.

https://travis-ci.org/bsipocz/gammapy/builds/111220871 checks whether it works on top of this branch.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 23, 2016

Member

I fixed the warning from docs/tutorials/npred/npred_convolved_significance.py via 14650d2 and 0d765ac.

Member

cdeil commented Feb 23, 2016

I fixed the warning from docs/tutorials/npred/npred_convolved_significance.py via 14650d2 and 0d765ac.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Feb 23, 2016

Member

I'm merging this now.

There's some more remaining cleanup to be done, but the diff is already very large (100 files touched) and Johannes is working on other cleanup in parallel.

Member

cdeil commented Feb 23, 2016

I'm merging this now.

There's some more remaining cleanup to be done, but the diff is already very large (100 files touched) and Johannes is working on other cleanup in parallel.

cdeil added a commit that referenced this pull request Feb 23, 2016

Merge pull request #462 from cdeil/update-ah-for-0.4
Cleanup for upcoming Gammapy 0.4 release

@cdeil cdeil merged commit 94c3541 into gammapy:master Feb 23, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@cdeil cdeil changed the title from Cleanup for upcoming Gammapy 0.4 release to Misc cleanup Apr 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment