simpeg / geoana

Interactive geoscience (mostly) analytic functions.
MIT License
22 stars 9 forks source link

bug fix for import error #22

Closed sgkang closed 3 years ago

sgkang commented 3 years ago

import error in \geoana\geoana\kernels\_extensions.

also, it seems the current conda version does not include latest implementations in the above folder. Am I right @jcapriot ?

jcapriot commented 3 years ago

What are you trying to do? You should be able to directly import the functions. You shouldn’t be directly importing anything from _extensions.

The geoana package should make the choice for you.

sgkang commented 3 years ago

I wanted to make sure that I am using the cythonized rTE.pyx, but it was not the case without the fix that I made ... Thought you still need an access to _extensions since you are importing that function in tranverse_electric_reflections.py

try:
    from geoana.kernels._extensions.rTE import rTE_forward, rTE_gradient
except ImportError:
jcapriot commented 3 years ago

What error does this line give you then?

from geoana.kernels._extensions.rTE import rTE_forward, rTE_gradient

sgkang commented 3 years ago

It says there is no modules name rTE.

jcapriot commented 3 years ago

It says there is no modules name rTE.

And you installed the package through conda-forge, hmm I’ll have to double check what exactly is coming down from there then on the installation. It’s likely a packaging issue then.

jcapriot commented 3 years ago

I would suggest to use the kernel imported from the non-hidden package. As even the numpy version is fairly fast on its own and the calling convention is identical.

codecov[bot] commented 3 years ago

Codecov Report

Merging #22 (d36de8a) into master (c446c2f) will increase coverage by 0.04%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage   59.28%   59.32%   +0.04%     
==========================================
  Files          20       20              
  Lines         948      949       +1     
==========================================
+ Hits          562      563       +1     
  Misses        386      386              
Impacted Files Coverage Δ
geoana/kernels/__init__.py 100.00% <100.00%> (ø)

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 c446c2f...d36de8a. Read the comment docs.

sgkang commented 3 years ago

I liked what you have done in the current version. We use the cythonized function if that works, if not we use numpy function. Could we keep that way?

jcapriot commented 3 years ago

That’s how it currently works if you import the rTE functions from geoana.kernels or geoana.kernels.transverse_electric_reflections.