Skip to content

update API entrypoint #1466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

update API entrypoint #1466

wants to merge 1 commit into from

Conversation

ancirTOR
Copy link

@ancirTOR ancirTOR commented Jun 1, 2022

new API entrypoint PVGIS 5.2

  • 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 and Milestone are assigned to the Pull Request and linked Issue.

new API entrypoint PVGIS 5.2
@mikofski
Copy link
Member

mikofski commented Jun 1, 2022

👀 @kanderso-nrel does pvgis have a new api endpoint?

@kandersolar
Copy link
Member

kandersolar commented Jun 1, 2022

See #1415. I had thought that we didn't need to change the url, and they would eventually point the original url to the new 5.2 data, but this page suggests otherwise:

NB The old entrypoint https://re.jrc.ec.europa.eu/api/tool_name?param1=value1&param2=value2&... will continue serve PVGIS 5.1 for a limited period.

https://joint-research-centre.ec.europa.eu/pvgis-photovoltaic-geographical-information-system/getting-started-pvgis/api-non-interactive-service_en

Anyway thanks @ancirTOR for the PR! Can you take a look at updating the pvgis tests for this change as well? I think we will also want some minor docstring edits for this as mentioned in #1415.

Edit: actually, does this maybe count as a breaking change? I know it's an external service that we don't control, but the above link says there's a way to keep using the old data. Does that mean we should stick with PVGIS 5.1 for now and wait until v0.10.0 to switch to PVGIS 5.2?

@kandersolar kandersolar added the io label Jun 1, 2022
@kandersolar kandersolar added this to the 0.9.2 milestone Jun 1, 2022
@AdamRJensen
Copy link
Member

I have a feeling that it is a bad idea to hardcode the version number. What is your opinion @kanderso-nrel?

As stated in the PVGIS doscs the endpoint https://re.jrc.ec.europa.eu/api/ will default to 5.1 for now:

The old entrypoint https://re.jrc.ec.europa.eu/api/tool_name?param1=value1&param2=value2&... will continue to serve PVGIS 5.1 for the time being.

To me, this means that sometime in the future they will change this to default to the PVGIS 5.2 database, i.e., generally to the most recent version with somedelay.

Having the version hardcoded makes it difficult for users to understand which database is actually being used. Some versions of pvlib will then default to 5.1, some will use 5.2, and in the future maybe also 5.3 and so forth. As it is set up right now, users can utilize a specific version of the database by manually specifying the URL:

data, input, meta = pvlib.iotools.get_pvgis_hourly(
    latitude=28.965734,
    longitude=117.147418,
    raddatabase='PVGIS-ERA5',
    url='https://re.jrc.ec.europa.eu/api/v5_2/')

@kandersolar
Copy link
Member

I guess "will continue serve PVGIS 5.1 for a limited period" and "will continue to serve PVGIS 5.1 for the time being" are both ambiguous -- they could mean that they will switch the old url to the new data, as you say and as I previously thought, or they could mean that they are changing the url scheme to explicitly include the data version, and someday the old versionless url will be retired. Maybe we could just ask them for clarity :)

@AdamRJensen
Copy link
Member

Hi @NikosAlexandris (employee at the Joint Research Center who works on PVGIS)
Perhaps you have an answer to the above question or could inform us who we can contact?
Thanks!

@ancirTOR
Copy link
Author

ancirTOR commented Jun 1, 2022

hi everyone, i will wait confirmation about this change before updating the pvgis test as well.

From what i can understand the current API entry point won't be supported anymore so everyone using it should update it to one of the newest releas 5.1 or 5.2.

Anyway it is a good point to ask directly to someone involved in the pvgis projet as AdamRJensen did.
I will be waiting for the confirmation.

@NikosAlexandris
Copy link

Apologies for the delay!

In the API Non-Interactive Service documentation page, it is stated

All the PVGIS tools can be accessed non-interactively using our web APIs. The entrypoints are:

PVGIS 5.1: https://re.jrc.ec.europa.eu/api/v5_1/tool_name?param1=value1&param2=value2&...

PVGIS 5.2: https://re.jrc.ec.europa.eu/api/v5_2/tool_name?param1=value1&param2=value2&...

NB The old entrypoint https://re.jrc.ec.europa.eu/api/tool_name?param1=value1&param2=value2&... will continue serve PVGIS 5.1 for a limited period.

Indeed, this isn't clear enough about the future setup. We are updating our database and once PVGIS v5.2 is ready, it will be the default version served via the stadard entrypoint https://re.jrc.ec.europa.eu/api/. What is served currently via https://re.jrc.ec.europa.eu/api/v5_2/ is beta and comes with issues mentioned in the release notes. In any case, we are not going to deprecate the default URL scheme.

@kandersolar
Copy link
Member

Thank you @NikosAlexandris for clarifying! Sounds like we don't need to make any changes on our side and can close this PR then.

Thanks also @ancirTOR for the PR, even if we didn't end up merging it :)

@kandersolar kandersolar closed this Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants