Feature/add germany berlin

[thiagovmdon]

Related to issue #62

Implements data fetcher for Germany-Berlin (Wasserportal Berlin) following the RiverDataFetcher base class.

Includes:

• Metadata scraping from Wasserportal (and cached data)
• Standardized coordinate transformation (UTM33N to WGS84)
• Addition of the module pyproj in requirements (useful here and in others for converting to WGS84)
• Daily variable support for stage, discharge, and temperature
• Example test script (just visual check) (5867601 gauge)

[kratzert]

Hi @thiagovmdon,

that looks great so far. A few things though:

• It would be great to have unit tests. Do you want me to add them or do you want to work on them?
• The fetcher needs to be linked to the docs. This requires a new rst file in docs/fetchers (see other countries) and then linking this new file to docs/api.rst. Afterwards, checking again if the docs compile and look reasonable (cd docs/ && make html). Afterwards you can open the docs from docs/build/html/index.html
• For metadata: I noticed that you only return the columns that are renamed. For other fetchers, I kept all columns and only renamed the once that match our global names. I think it would be good if you can change that as well.
• As for new fetchers that we add: Do you know about a) terms of use of the data and b) terms of use of the API (i.e. any limits on queries per second etc.?). Maybe we can already add this information for new fetchers to the class docstring.

Let me know on what you want to work or if I should take over this branch.

[thiagovmdon]

Hi @thiagovmdon,

that looks great so far. A few things though:

• It would be great to have unit tests. Do you want me to add them or do you want to work on them?
• The fetcher needs to be linked to the docs. This requires a new rst file in docs/fetchers (see other countries) and then linking this new file to docs/api.rst. Afterwards, checking again if the docs compile and look reasonable (cd docs/ && make html). Afterwards you can open the docs from docs/build/html/index.html
• For metadata: I noticed that you only return the columns that are renamed. For other fetchers, I kept all columns and only renamed the once that match our global names. I think it would be good if you can change that as well.
• As for new fetchers that we add: Do you know about a) terms of use of the data and b) terms of use of the API (i.e. any limits on queries per second etc.?). Maybe we can already add this information for new fetchers to the class docstring.

Let me know on what you want to work or if I should take over this branch.

Hi Freddy.

Thanks! I left with some parts for you to do, but I can also do myself the tests! I assume in the end you do a quick overview check, so no worries. I can take care of the points!

1. I can add them, no worries.
2. Aha, that is good to know! I can also have a look on that!
3. That is my bad. I assumed we would be returning only the chosen variables for the get_metadata function, but indeed I think it is more reasonable to return everything... For Argentina I saw already the need for a new column and also for others... I will adjust this. Sorry. Also, I might need to adjust other of the fetcher codes still in the discussion (or feel free to do so in case you start a new branch with them... it is just return all + rename the ones currently. Not too much of a deal).
4. Good point. For this one there are no limits stated, and the license is open (DL-DE-BY) as stated in our Google drive file. Do you want to add it in a specific way? like perhaps in the metadata? one column? Just in the docstring?

[kratzert]

I see. No worries, I can do the tests, which is a trivial thing to add given the current state of your code. No worries to spend time understanding the test structure. I'll wait for you to finish the other tasks, then I add the test.

Re terms of use / license: I have no good plan yet. For Norway, see #70 I added simple a link to their documentation to the doc string which then appears in the docs, see https://rivretrieve-python.readthedocs.io/en/latest/fetchers/norway.html For now, I think it is enough if we include the information that we have, maybe with a link, to the doc-string. Later, we can see if we want to show this data differently.

Re docs: The changes required a trivial, but I will let you look into this, as this is maybe a good way to understand the sphinx doc structure and to test-compile them locally, if you haven't done so before. Could be helpful in the future to know how to manually work on the docs.

[thiagovmdon]

I see. No worries, I can do the tests, which is a trivial thing to add given the current state of your code. No worries to spend time understanding the test structure. I'll wait for you to finish the other tasks, then I add the test.

Re terms of use / license: I have no good plan yet. For Norway, see #70 I added simple a link to their documentation to the doc string which then appears in the docs, see https://rivretrieve-python.readthedocs.io/en/latest/fetchers/norway.html For now, I think it is enough if we include the information that we have, maybe with a link, to the doc-string. Later, we can see if we want to show this data differently.

Re docs: The changes required a trivial, but I will let you look into this, as this is maybe a good way to understand the sphinx doc structure and to test-compile them locally, if you haven't done so before. Could be helpful in the future to know how to manually work on the docs.

Awesome. All clear then. I will make the changes, and about the docs it is exactly what I thought: It will be very helpful eventually. I will let you know as soon as they're made.

[thiagovmdon]

I see. No worries, I can do the tests, which is a trivial thing to add given the current state of your code. No worries to spend time understanding the test structure. I'll wait for you to finish the other tasks, then I add the test.

Re terms of use / license: I have no good plan yet. For Norway, see #70 I added simple a link to their documentation to the doc string which then appears in the docs, see https://rivretrieve-python.readthedocs.io/en/latest/fetchers/norway.html For now, I think it is enough if we include the information that we have, maybe with a link, to the doc-string. Later, we can see if we want to show this data differently.

Re docs: The changes required a trivial, but I will let you look into this, as this is maybe a good way to understand the sphinx doc structure and to test-compile them locally, if you haven't done so before. Could be helpful in the future to know how to manually work on the docs.

Hi Freddy. I think it is all right now. Let me know if I missed something. I will update the Korean branch accordingly as well. The website works fine, and we could perhaps think of a standardize way to write the docstrings. I liked the way you put, but for Germany I just added a link tot he documentation too. where there is also the details about how to use the API. regarding the license, it does not state things about api limits, but it points to the official data license.

[kratzert]

@thiagovmdon I added the unit test. Just two things I noticed:

The script that you added to tests/ was the script I usually put under examples/ (i.e. a small script that allows the user to quickly download and plot the timeseries for one station). While the unittest is intented to test the implemented code behavior.

The other thing is that for .rst files, it is important that the underlining of the heading is exactly as long as the title. If that is not the case, the page is not rendered in the docs. I actually thought that it would also raise an error when building the docs, I think this is at least what happened in the past. The only way how I noticed it now is that when I built the docs offline and opened the index.html in the browser, I saw that the Berlin fetcher was missing.

I will now go ahead and merge this into main

[thiagovmdon]

Hi @kratzert, about the test it was my bad! I did the test locally and forgot to ignore it... Sorry.

About the docs, I had the impression that it worked well locally when I opened it here. Perhaps the problem was when I switched the order to put the Berliner alphabetically? For the next I will keep this length constraint into consideration. Thanks for the input.