spa_c_files don't build with VC14 (Python 3.5)

[mikofski]

windows 7 amd64
Cython (0.24)
Python 3.5.1 (v3.5.1:37a07cee5969, Dec 6 2015, 01:54:25) [MSC v.1900 64 bit (AMD64)] on win32

if I try to build spa_c for Python-3.5 I get the following:

$ py -3.5 setup.py build_ext --inplace
running build_ext
building 'spa_py' extension
creating build
creating build\temp.win-amd64-3.5
creating build\temp.win-amd64-3.5\Release
C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\BIN\amd64\cl.exe /c /nologo /Ox /W3 /GL /DNDEBUG /MD -IC:\Python35\include -IC:\Python35\include "-IC:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\INCLUDE" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\shared" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\um" "-IC:\Program Files (x86)\Windows Kits\8.1\include\\winrt" /Tcspa_py.c /Fobuild\temp.win-amd64-3.5\Release\spa_py.obj
spa_py.c
c:\users\mmikofski\documents\projects\pvlib-python\pvlib\spa_c_files\spa.h(88): error C2032: '__timezone': function cannot be member of struct '<unnamed-tag>'
spa_py.c(854): error C2059: syntax error: '('
spa_py.c(1700): error C2059: syntax error: '('
spa_py.c(1702): error C2275: 'PyObject': illegal use of this type as an expression
c:\python35\include\object.h(110): note: see declaration of 'PyObject'
spa_py.c(1702): error C2059: syntax error: ')'
spa_py.c(1702): error C2223: left of '->ob_refcnt' must point to struct/union
spa_py.c(1702): error C2065: '_py_decref_tmp': undeclared identifier
spa_py.c(1702): warning C4312: 'type cast': conversion from 'int' to 'PyObject *' of greater size
error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\BIN\\amd64\\cl.exe' failed with exit status 2

[wholmgren]

It's been a long time since I used spa_c, and I don't have any experience with Visual Studio, so I don't have any hints for you there. However, I'll point out that the benchmarks in #48 show that spa_python+numpy and spa_c are essentially the same speed, and spa_python+numba is significantly faster than spa_c. As with many benchmarks, your mileage may vary.

[wholmgren]

@mikofski ok to close this? while it may still be a bug with pvlib it may now be irrelevant after 2 years of default tooling changes

[mikofski]

No I'm still looking into it, I just ran it again in a fresh Python-3.7 environment, and I still get the exact same error, so no I don't think we should close it just yet, b/c we need to consider if we want to include spa-c-files at all, if we close this issue, then we should remove the folder, and be done. Maybe this can become a standalone project, perhaps I can add it to SolarUtils and try using ctypes instead of cython?

One thing, we can definitely delete the spa_py.c file from the repository, as this is a cython generated file, and was probably committed by accident.

[wholmgren]

Thanks for checking Mark. I'd be happy to see it move to another repository. We could still keep the wrapper option in pvlib.solarposition.py just like we do for pyephem.

I believe we include spa_py.c because it allows people to compile the package with only a c compiler and not cython.

[mikofski]

I don't think that works, that file spa_py.c is generated by cython and requires cython to compile. Anyway, it doesn't work.

I just compiled spa.c, spa.h, and spa_tester.c to spa_tester.exe using visual studio 2015 (which AFAIK is used on Windows for Python >3.5) and it really does work,

>cl /Fobuild\ /Febuild\spa_tester.exe spa.c spa_tester.c
Microsoft (R) C/C++ Optimizing Compiler Version 19.15.26726 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

spa.c
spa_tester.c
Generating Code...
Microsoft (R) Incremental Linker Version 14.15.26726.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:build\spa_tester.exe
build\spa.obj
build\spa_tester.obj



>build\spa_tester.exe
Julian Day:    2452930.312847
L:             2.401826e+01 degrees
B:             -1.011219e-04 degrees
R:             0.996542 AU
H:             11.105902 degrees
Delta Psi:     -3.998404e-03 degrees
Delta Epsilon: 1.666568e-03 degrees
Epsilon:       23.440465 degrees
Zenith:        50.111622 degrees
Azimuth:       194.340241 degrees
Incidence:     25.187000 degrees
Sunrise:       06:12:43 Local Time
Sunset:        17:20:19 Local Time

so I think the issue is in the supporting Cython code: cspa_py.pxd and spa_py.pyx, maybe I can figure out (haven't yet), but It might just be easier to drop cython and just use ctypes or cffi, if cython is the issue. Anyway, that would be a concern for what ever project then picked this up, if you decided to boot it, which I'm in favor of to expedite closure of this issue. 😉

[wholmgren]

It was supposed to work like this from the cython docs.

Maybe the upcoming SSC release will make this wrapper pointless.

I say we continue to ignore it for now.

[mikofski]

OK, I figured it out, but I'll never understand why. For some reason, Cython does not like the keyword timezone which is okay with VisualStudio but not Python.

Replacing timezone with time_zone everywhere solves the problem

[wholmgren]

How did you find that???

Do you want to sneak this into 0.6.0? I'm going to let #575 sit until tomorrow.

[mikofski]

• Done, but how do you want to test that this actually works?
• Also, I don't think my fix is safe for python-2.7 is that okay?

[wholmgren]

Done, but how do you want to test that this actually works?

take your word on it

Also, I don't think my fix is safe for python-2.7 is that okay?

ok with me, but update the readme in that subpackage accordingly.

[mikofski]

here's the test output, LGTM:

>>> spa_calc(year=2004, month=10, day=17, hour=12, minute=30, second=30, time_zone=-7, longitude=-105.1786, latitude=39.742476, elevation=1830.14, pressure=820, temperature=11, delta_t=67)

{'year': 2004,
 'month': 10,
 'day': 17,
 'hour': 12,
 'minute': 30,
 'second': 30.0,
 'delta_ut1': 0.0,
 'delta_t': 67.0,
 'time_zone': -7.0,
 'longitude': -105.1786,
 'latitude': 39.742476,
 'elevation': 1830.14,
 'pressure': 820.0,
 'temperature': 11.0,
 'slope': 30.0,
 'azm_rotation': -10.0,
 'atmos_refract': 0.5667,
 'function': -1984328928,
 'e0': 39.59209464796398,
 'e': 39.60858878898177,
 'zenith': 50.39141121101823,
 'azimuth_astro': 14.311961805946808,
 'azimuth': 194.3119618059468,
 'incidence': 6.95264798328736e-310,
 'suntransit': 8.648152507504e-312,
 'sunrise': 1.6578093e-316,
 'sunset': 2.67e-322}

[solarjoe]

@mikofski

I have exactly the same problem, but I don't think that Cython is to blame. I am using cffi in combination with the original spa.h and spa.c, so without Cython.

Any idea what might be the reason then?

[solarjoe]

Nevermind, found it :)

https://bugs.python.org/issue24643