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

Re-write of colour fix code #21

Merged
merged 6 commits into from
Jul 12, 2019
Merged

Conversation

williamjameshandley
Copy link
Collaborator

@williamjameshandley williamjameshandley commented Jul 11, 2019

@lukashergt: is there any reason why this wouldn't fix #19 in the same manner as #15?

I think this way of doing it is slightly more logical, and will be easier for downstream changes to integrate.

Checklist:

  • I have performed a self-review of my own code
  • My code is PEP8 compliant (flake8 anesthetic tests)
  • My code contains compliant docstrings (pydocstyle --convention=numpy anesthetic)
  • New and existing unit tests pass locally with my changes (python -m pytest)
  • I have added tests that prove my fix is effective or that my feature works

if xmin is not None and (min(d) - xmin)/(max(d)-min(d)) > 1e-2:
xmin = None
if xmax is not None and (xmax - max(d))/(max(d)-min(d)) > 1e-2:
xmax = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm getting this error now:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-26-3db102adf145> in <module>()
----> 1 fig, axes= pc_lcdm.plot_2d(['A_s', 'n_s', 'r'], label='LCDM')

/scratch/.virtualenvs/py27env/lib/python2.7/site-packages/anesthetic/samples.pyc in plot_2d(self, axes, *args, **kwargs)
    278                     ax_ = ax.twin if x == y else ax
    279                     self.plot(ax_, x, y, plot_type=types[pos], *args,
--> 280                               **local_kwargs[pos])
    281 
    282         return fig, axes

/scratch/.virtualenvs/py27env/lib/python2.7/site-packages/anesthetic/samples.pyc in plot(self, ax, paramname_x, paramname_y, *args, **kwargs)
    155 
    156             return plot(ax, x, y, xmin=xmin, xmax=xmax, ymin=ymin, ymax=ymax,
--> 157                         *args, **kwargs)
    158 
    159     def plot_1d(self, axes, *args, **kwargs):

/scratch/.virtualenvs/py27env/lib/python2.7/site-packages/anesthetic/plot.pyc in contour_plot_2d(ax, data_x, data_y, *args, **kwargs)
    351 
    352     x, y, pdf = kde_2d(data_x, data_y,
--> 353                        xmin=xmin, xmax=xmax, ymin=ymin, ymax=ymax)
    354     pdf /= pdf.max()
    355     p = sorted(pdf.flatten())

/scratch/.virtualenvs/py27env/lib/python2.7/site-packages/anesthetic/kde.pyc in kde_2d(d_x, d_y, xmin, xmax, ymin, ymax)
     79     f = [xmax is None or xmin is None,
     80          ymax is None or ymin is None]
---> 81     d_x_, d_y_ = mirror_2d(d_x, d_y, xmin, xmax, ymin, ymax)
     82 
     83     with warnings.catch_warnings():

/scratch/.virtualenvs/py27env/lib/python2.7/site-packages/anesthetic/utils.pyc in mirror_2d(d_x_, d_y_, xmin, xmax, ymin, ymax)
     79 def mirror_2d(d_x_, d_y_, xmin=None, xmax=None, ymin=None, ymax=None):
     80     """If necessary apply reflecting boundary conditions."""
---> 81     d_x = d_x_.copy()
     82     d_y = d_y_.copy()
     83 

AttributeError: 'list' object has no attribute 'copy'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How easy would it be to add a failing test which shows this error?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Test added.

The test for samples.plot_2d didn't check for 1d lists as input.

@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

Merging #21 into master will increase coverage by 0.29%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #21      +/-   ##
=========================================
+ Coverage      87%   87.3%   +0.29%     
=========================================
  Files          15      15              
  Lines        1093    1095       +2     
=========================================
+ Hits          951     956       +5     
+ Misses        142     139       -3
Impacted Files Coverage Δ
anesthetic/utils.py 90.09% <100%> (+0.09%) ⬆️
anesthetic/samples.py 88.23% <91.3%> (+1.84%) ⬆️

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 ba7b3b6...147b3e4. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

Merging #21 into master will increase coverage by 0.29%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #21      +/-   ##
=========================================
+ Coverage      87%   87.3%   +0.29%     
=========================================
  Files          15      15              
  Lines        1093    1095       +2     
=========================================
+ Hits          951     956       +5     
+ Misses        142     139       -3
Impacted Files Coverage Δ
anesthetic/utils.py 90.09% <100%> (+0.09%) ⬆️
anesthetic/samples.py 88.23% <91.3%> (+1.84%) ⬆️

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 ba7b3b6...147b3e4. Read the comment docs.

@williamjameshandley williamjameshandley added this to the 1.1 milestone Jul 12, 2019
Lukas Hergt and others added 4 commits July 12, 2019 12:03
* At the moment `test_different_parameters` only tests for `plot_2d`
  with 2d input for `axes`, i.e. `[params_x, params_y]` as input.
  Ordinary triangle plots are generated with 1d input though, i.e.
  `params_x` as input.

* This is particularly important here, when `params_x` has more
  parameters than the model tested which is failing at the moment.

Changes to be committed:
* modified:   tests/test_samples.py
@williamjameshandley
Copy link
Collaborator Author

@lukashergt I believe this is now fixed, although would be good to check that you don't get any further issues arising on your system.

@williamjameshandley williamjameshandley merged commit a2d6edf into master Jul 12, 2019
@williamjameshandley williamjameshandley deleted the rewrite_colour_fix branch July 12, 2019 14:14
lukashergt pushed a commit to lukashergt/anesthetic that referenced this pull request Jul 14, 2019
* In handley-lab#21 the calls of `ax.plot([], [])` and similar were changed to
  setting `x = []` and passing it to the plotting functions.
  - This does not take care of the different colour cycles of plot and
    scatter and thus does not work.

Changes to be committed:
* modified:   anesthetic/samples.py
williamjameshandley pushed a commit that referenced this pull request Jul 15, 2019
* revert rewrite of colour fix from #21

* In #21 the calls of `ax.plot([], [])` and similar were changed to
  setting `x = []` and passing it to the plotting functions.
  - This does not take care of the different colour cycles of plot and
    scatter and thus does not work.

Changes to be committed:
* modified:   anesthetic/samples.py

* make pep8 conform
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

Successfully merging this pull request may close these issues.

wrong colour when second model plots into empty axes
2 participants