trichter / rf

Receiver function calculation in seismology

MIT License

106 stars 62 forks source link

Review - windows compatibility made easier :) #24

Closed ThomasLecocq closed 4 years ago

ThomasLecocq commented 4 years ago

review for https://github.com/openjournals/joss-reviews/issues/1808

I made a small change to the documentation & travis CI config so normally windows could also be tested with fortran/toeplitz.

codecov-io commented 4 years ago

Codecov Report

Merging #24 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #24   +/-   ##
=======================================
  Coverage   90.71%   90.71%           
=======================================
  Files          17       17           
  Lines        1982     1982           
=======================================
  Hits         1798     1798           
  Misses        184      184

Impacted Files	Coverage Δ
rf/__init__.py	`85.71% <ø> (ø)`	:arrow_up:

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 360601a...5c7d2cc. Read the comment docs.

trichter commented 4 years ago

@ThomasLecocq, thanks for this insight! The toeplitz package installs successfully on travis. Unfortunately, the import is not yet working.

https://travis-ci.org/trichter/rf/jobs/656156204#L658

Could you test the toeplitz package locally on your Windows machine?

pip install toeplitz
toeplitz-runtests

ThomasLecocq commented 4 years ago

hey

toeplitz-runtests

doesn't exist...

(rfenv) λ python
Python 3.6.7 (default, Dec  6 2019, 07:03:06) [MSC v.1900 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import toeplitz
>>> toeplitz
<module 'toeplitz' from 'c:\\ProgramData\\Anaconda3\\envs\\rfenv\\lib\\site-packages\\toeplitz.cp36-win_amd64.pyd'>

seem OK

edit: I'm on py36, testing now on py37 !

trichter commented 4 years ago

That is strange, maybe you need to restart your console for finding this command line script? But it is not so important. Does

from toeplitz import sto_sl, cto_sl

also work?

ThomasLecocq commented 4 years ago

yes, and just now OK on py3.7 too

ThomasLecocq commented 4 years ago

C:\Users\TLecocq\Cmder
(rfenv37) λ python C:\ProgramData\Anaconda3\envs\rfenv37\Scripts\toeplitz-runtests
..
----------------------------------------------------------------------
Ran 2 tests in 0.037s

OK

trichter commented 4 years ago

OK, in this case I will merge your PR and investigate the travis problem directly in the toeplitz repository.

Thanks again!