Adding PVGIS horizon data retrieval method

[bgpierc]

• Closes Add retrieval function for horizon profile from MINES Paris Tech #1295, Calculating horizon profiles and associated shading losses #758
• I am familiar with the contributing guidelines
• Tests added
• Updates entries to docs/sphinx/source/api.rst 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 and Milestone are assigned to the Pull Request and linked Issue.

This PR adds a function to fetch a horizon profile from PVGIS.

In this request, I've added two main functions: one in a new module horizon.py that, given an SRTM DEM, calculates a horizon profile in terms of (azimuth, elevation) manually, and an addition in iotools that fetches the same from pvgis (#1295 ). I also incorporated a few functions from abandoned #758. I've tested that they're compatible, but have not verified the results.

Along with #1295, this gives pvlib 3 potential ways of retrieving the horizon profile. I'm going to be testing them against each other and collected field data like @mikofski suggested in #1295 . I'll add more functionality as I go along but I wanted to check in with @cwhanse and @jsstein with what we have now. I'm currently working on retrieving raw SRTM data automatically, as well.

[cwhanse]

@bgpierc before we start polishing in earnest, could you clear up the stickler items? Makes it much easier to review. My first reactions are 1) that many of the utility functions in horizon.py would be better as private, and 2) several added dependencies should be optional.

[bgpierc]

@cwhanse fixed stickler errors, and added underscores for private functions. How should I specify optional dependencies?

[kandersolar]

Thanks @bgpierc!

How should I specify optional dependencies?

Add an entry for each new dependency here: https://github.com/pvlib/pvlib-python/blob/master/setup.py#L56-L58

And then, rather than importing the optional dependencies at the top of the module like normal (which, when the new functions are eventually added to an __init__.py somewhere, will cause import pvlib to fail if the optional deps are not available), import them inside the functions that use them like this: https://github.com/pvlib/pvlib-python/blob/master/pvlib/bifacial.py#L95

[bgpierc]

Perfect, I added new dependencies to the optional section. I also changed the horizon_map function to accept an array for the DEM rather then a path, as there's a few different file types (GeoTiff, ascii, .hgt) that vary depending on where you get the data from. I'm currently working on an automated download function for DEMs, so I was meaning to change that anyway.

[cwhanse]

@bgpierc mostly docstring stuff as I am getting familiar with the new code.

I support adding the DEM functions to a new module horizon.py. Perhaps more of the DEM functions could become private, unless you think that a user will need them to work with DEM files in their code.

I think the three public functions from JPalakapillyKWH's PR should go to shading.py.

The rest of pvlib users latitude and longitude so it would be better use these terms here, required for public functions but not for private functions.

[cwhanse]

Can we add the unit and sign convention? I assume latitude, longitude are degrees.decimal degrees and negative longitude is west of the prime meridian. This info may be better in a Notes section than in the description of the output.

Suggested change

	Tuple of np.array of latitude,longitude
	corresponding to the pixels in the GeoTiff
	Tuple (latitude, longitude) corresponding to the pixels in
	the GeoTiff. Each of latitude and longitude is a numpy.array.
	Notes
	------
	Latitude and longitude are...

[bgpierc]

Yep, your assumption is correct, I added it to be explicit.

[cwhanse]

Here, azimuth will include both 0 and 360. I haven't looked closely below to see if the duplicate is removed. If it isn't, I'd drop the last value 360 here before moving forward.

Suggested change

	azimuth = np.linspace(0, 360, az_len)
	azimuth = np.linspace(0, 360, num=az_len)

[bgpierc]

Oops, off-by-one. Good catch. Changed to be in range [0,360)

[AdamRJensen]

Given that this is an iotools function, it could be considered following the convention of returning a tuple of (data, meta). For this function, the meta object would just be an empty dictionary.

If we can imagine that other horizon fetching functions could return metadata then I would vote for doing this.

[kandersolar]

For this function, the meta object would just be an empty dictionary.

Well, the PVGIS API does actually return metadata, we just aren't doing anything with it right now. I'm in favor of having this function follow the data, metadata convention and passing through the metadata returned by the API. Good idea!

[bgpierc]

Ok, I had it return the meta data from PVGIS as well. I'm not sure if we want to make the function return a Series rather the a DataFrame as the other pvgis functions all return a DataFrame. Also, I think having the horizon_azimuth be explicit as the index column is helpful for clarity.

[kandersolar]

Looks great to me! Thanks @bgpierc, @AdamRJensen, and the long list of other contributors here :)

CI failure is an unrelated codecov upload issue and fine to ignore.