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

Replacing matplotlib.mlab.griddata by scipy.interpolate.griddata #148

Merged
merged 8 commits into from Dec 9, 2014

Conversation

Projects
None yet
3 participants
@hbueno
Member

hbueno commented Nov 24, 2014

As discussed in issue #146, I'd some problems when I tried to run PrismMesh.carvetopo because of matplotlib.mlab.griddata. So, @leouieda suggested to replace the matplotlib.mlab.griddata functions by scipy.interpolate.griddata.

@leouieda I don't know what's the best interpolation method to set as default. With scipy.interpolate.griddata our options are linear, nearest and cubic. Here I'm using cubic.

Checklist:

  • Make tests for new code
  • Create/update docstrings
  • Code follows PEP8 style conventions
  • Code and docs have been spellchecked
  • Changelog entry
  • Include new dependencies in docs, requirements.txt, README
  • Documentation builds properly
  • All tests pass
  • Can be merged
@leouieda

This comment has been minimized.

Member

leouieda commented Nov 24, 2014

Thanks for this Henrique!

@leouieda I don't know what's the best interpolation method to set as default. With scipy.interpolate.griddata our options are linear, nearest and cubic. Here I'm using cubic.

Cubic is fine.

Please replace the mlab call in fatiando.gridder and any other places you can find. I think that is the only one though. To make sure, check that all tests and recipes run on you machine.

@leouieda leouieda added the bug label Nov 24, 2014

@leouieda leouieda modified the milestones: 1.0, 0.4 Nov 24, 2014

@hbueno

This comment has been minimized.

Member

hbueno commented Nov 24, 2014

@leouieda, please is there any special application for the nearest neighbor method 'nn' of mlab?
If not, I think we can remove it from the gridder.py#L248 and use only the nearest of scipy.interpolate.griddata.

@eusoubrasileiro

This comment has been minimized.

Contributor

eusoubrasileiro commented Nov 25, 2014

Good bye 'nearest'. We say farewell
On Nov 24, 2014 8:20 PM, "Henrique Bueno dos Santos" <
notifications@github.com> wrote:

@leouieda https://github.com/leouieda, please is there any special
application for the nearest neighbor method 'nn' of mlab?
If not, I think we can remove it from the gridder.py#L248
https://github.com/fatiando/fatiando/blob/master/fatiando/gridder.py#L248
and use only the nearest of scipy.interpolate.griddata.


Reply to this email directly or view it on GitHub
#148 (comment).

@eusoubrasileiro

This comment has been minimized.

Contributor

eusoubrasileiro commented Nov 25, 2014

Ops farewell to the wrong defunct.
Ops two defunct with same name and just different nicknames.

Cheers fatiandovillage
On Nov 25, 2014 8:51 AM, "André" eusoubrasileiro@gmail.com wrote:

Good bye 'nearest'. We say farewell
On Nov 24, 2014 8:20 PM, "Henrique Bueno dos Santos" <
notifications@github.com> wrote:

@leouieda https://github.com/leouieda, please is there any special
application for the nearest neighbor method 'nn' of mlab?
If not, I think we can remove it from the gridder.py#L248
https://github.com/fatiando/fatiando/blob/master/fatiando/gridder.py#L248
and use only the nearest of scipy.interpolate.griddata.


Reply to this email directly or view it on GitHub
#148 (comment).

@leouieda

This comment has been minimized.

Member

leouieda commented Nov 25, 2014

@hbueno you can tick the "Update docstrings" task (you've already update the interp docstring).

You should leave the changelog for last. I've update the develop docs putting it last in the checklist.
The reason for that is that the changelog almost always causes merge conflicts (like the one you're having now). It happens because master updated the changelog at the same time you did. To avoid that, merge the master branch into yours before editing the changelog. For now, you'll have to update your master and merge it into your branch:

git checkout master
git pull https://github.com/fatiando/fatiando.git master
git checkout YOUR_BRANCH
git merge master

When you run this now you'll have "merge conflicts". You'll need to resolve them before we can merge the PR. See https://help.github.com/articles/resolving-a-merge-conflict-from-the-command-line/ for a guide on how to do that.

Merge branch 'master' into griddata-carvetopo
Conflicts:
	doc/changelog.rst
@leouieda

This comment has been minimized.

leouieda commented on doc/changelog.rst in eeed755 Dec 9, 2014

@hbueno please add that you replaced matplotlib with scipy in fatiando.mesher.PrismMesh.carvetopo as well.

@leouieda

This comment has been minimized.

Member

leouieda commented Dec 9, 2014

@hbueno great job! After this one little fix I think it's ready to merge.

@leouieda

This comment has been minimized.

Member

leouieda commented Dec 9, 2014

@hbueno looks good to merge. Do you have anything else to add?

@hbueno

This comment has been minimized.

Member

hbueno commented Dec 9, 2014

No. Thanks...

leouieda added a commit that referenced this pull request Dec 9, 2014

Merge pull request #148 from hbueno/griddata-carvetopo
Replacing matplotlib.mlab.griddata by scipy.interpolate.griddata

@leouieda leouieda merged commit 45c93b1 into fatiando:master Dec 9, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment