Simplify and future-proof numba import in spa.py

[kandersolar]

• [ ] Closes #xxxx
• I am familiar with the contributing guidelines
• Tests added
• Updates entries in docs/sphinx/source/reference for API changes.
• Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
• New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
• Pull request is nearly complete and ready for detailed review.
• Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

spa.py currently inspects numba's version string to determine whether it meets the required minimum of 0.17.0:

pvlib-python/pvlib/spa.py

Lines 23 to 40 in 1601760

	try:
	from numba import jit, __version__
	except ImportError:
	warnings.warn('Could not import numba, falling back to numpy ' +
	'calculation')
	jcompile = nocompile
	USE_NUMBA = False
	else:
	major, minor = __version__.split('.')[:2]
	if int(major + minor) >= 17:
	# need at least numba >= 0.17.0
	jcompile = jit
	USE_NUMBA = True
	else:
	warnings.warn('Numba version must be >= 0.17.0, falling back to ' +
	'numpy')
	jcompile = nocompile
	USE_NUMBA = False

However, the way it performs the check isn't quite correct. Consider what will happen when numba.__version__ eventually ticks over from 0.x.y to 1.0.0. That code on line 32 will compare int("1" + "0") with 17 and incorrectly decide that the installed version of numba isn't new enough.

Rather than fix the version check, I think it makes sense to just remove that check in spa.py altogether in favor of specifying the minimum in setup.py as usual. Even that is probably unnecessarily cautious; numba 0.17.0 is ancient and I think it would be safe to assume that any version of numba that pvlib might encounter today would be much more recent than that, but no harm in setting the minimum anyway.

[kandersolar]

To-do: make the same change to has_numba in conftest.py

[AdamRJensen]

I'm all for simplifying things!

LGTM

[AdamRJensen]

Suggested change

	if os.getenv('PVLIB_USE_NUMBA', '0') != '0':
	if os.getenv('PVLIB_USE_NUMBA') is not None:

[kandersolar]

I think this change would require some corresponding changes in the part of solarposition.py that controls how spa.py is imported (by setting this environment variable to '0' or '1'), so I'm inclined to leave this as-is for now. It will be addressed someday with #1060.

[AdamRJensen]

I didn't realize that it could be defined as zero (I just assumed it would be undefined). Fine with me to just leave it.

[AdamRJensen]

Is there a reason for inconsistent usage of upper vs. lower case, e.g., USE_NUMBA vs. has_numba?

[kandersolar]

The reason I see is is that USE_NUMBA is a module-level variable used inside a function far away from where it is defined while has_numba is used once in the same scope immediately after its definition. But of course it's just style :)

[AdamRJensen]

Thanks @kandersolar for your continuous commitment to keeping pvlib in good shape!