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

Seismic vis (Obspy Integration) #192

Merged
merged 8 commits into from Apr 30, 2015

Conversation

Projects
None yet
2 participants
@eusoubrasileiro
Contributor

eusoubrasileiro commented Apr 15, 2015

Reopening PR 61 and PR 126 Obspy integration.

Simpler cookbook example where Obspy is just imported.

screenshot from 2015-04-15 15 23 26

  • Make tests for new code (ok then)
  • Create/update doc-strings
  • Update requirements
  • Code follows PEP8 style conventions
  • Code and docs have been spell-checked (perfect)
  • Changelog entry (doing now)
  • Documentation builds properly (builds properly)
  • All tests pass
  • Can be merged

eusoubrasileiro added some commits Sep 22, 2014

seismic-vis ready for matrices.
Cookbook uses Obspy for reading a SEGY example file.
seismic-vis ready for matrices reviewed
Cookbook uses Obspy for reading a SEGY example file.
seismic-vis PEP8 ok
few changes on cookbook font style
@leouieda

This comment has been minimized.

Member

leouieda commented Apr 16, 2015

👍

@eusoubrasileiro

This comment has been minimized.

Contributor

eusoubrasileiro commented Apr 16, 2015

Rsrs I got it hahah

@@ -0,0 +1,37 @@
"""
Seismic: seismic plotting utilities uses Obspy for SEGY reading

This comment has been minimized.

@leouieda

leouieda Apr 24, 2015

Member

@eusoubrasileiro could you change the title to something like "plotting a seismic section"? The title should be something that a person will search for. Just to be clear, most of my titles for recipes are awful and need to be changed.

This comment has been minimized.

@eusoubrasileiro

eusoubrasileiro Apr 26, 2015

Contributor

ok got it.

url = "http://dl.dropboxusercontent.com/" \
"s/i287ci4ww3w7gdt/marmousi_nearoffset.segy"
urllib.urlretrieve(url, 'marmousi_nearoffset.segy')
segyfile = segy.readSEGY('marmousi_nearoffset.segy')

This comment has been minimized.

@leouieda

leouieda Apr 24, 2015

Member

Could you add a comment above this line saying: "We'll use the obspy library to load the SEGY data"

This comment has been minimized.

@eusoubrasileiro

eusoubrasileiro Apr 30, 2015

Contributor

sure, done

color='k', normalize=False):
"""
Plot a seismic section (numpy 2D array matrix) as wiggles.
Slow for more than 200 traces, in this case use `seismic_image`.

This comment has been minimized.

@leouieda

leouieda Apr 24, 2015

Member

Add a line between these two sentences and make the second line a warning with:

Plot a seismic section (numpy 2D array matrix) as wiggles.

.. warning::        
    Slow for more than 200 traces, in this case use ``seismic_image``.

Notice that the format for code in rst is ``(not` in Markdown). I do this all the time.

@leouieda

This comment has been minimized.

Member

leouieda commented Apr 24, 2015

@eusoubrasileiro don't worry about the tests for now. We need to get all of fatiando.vis tested eventually but this will need a larger effort.

@leouieda

This comment has been minimized.

Member

leouieda commented Apr 24, 2015

@eusoubrasileiro sorry I didn't the comment about tesseroids. The tesseroids tests are a bit slow but not as much as you're experiencing. The new code uses numba for speedups. If it's not installed, it runs the slow numpy version instead. So install numba with conda install numba and you'll see a very large speedup in the tests.

@leouieda

This comment has been minimized.

Member

leouieda commented Apr 24, 2015

You can tick off the requirements item as well.

@eusoubrasileiro

This comment has been minimized.

Contributor

eusoubrasileiro commented Apr 26, 2015

@leouieda It`s done.

"""
Seismic: plotting a seismic section from a SEGY
Uses ObsPy package for SEGY file reading

This comment has been minimized.

@leouieda

leouieda Apr 27, 2015

Member

@eusoubrasileiro final thing, I promise! Could you also add a line here saying something like "you can find instructions for installing Obspy at http://obspy.org/"?

This comment has been minimized.

@eusoubrasileiro

eusoubrasileiro Apr 30, 2015

Contributor

Sure, done

@leouieda

This comment has been minimized.

Member

leouieda commented Apr 30, 2015

@eusoubrasileiro perfect!

leouieda added a commit that referenced this pull request Apr 30, 2015

Merge pull request #192 from eusoubrasileiro/seismic-vis
Seismic vis (Obspy Integration)

@leouieda leouieda merged commit cab653f into fatiando:master Apr 30, 2015

1 check passed

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

@eusoubrasileiro eusoubrasileiro deleted the eusoubrasileiro:seismic-vis branch Apr 30, 2015

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