Ingenjorsarbete-For-Klimatet / ifk-smhi

SMHI climate data client.
https://ingenjorsarbeteforklimatet.se/ifk-smhi/
MIT License
8 stars 1 forks source link

Features/smhi root #35

Closed docNord closed 1 year ago

docNord commented 1 year ago

I figure it is time to merge this into main now, to keep the pull request reasonably sized.

Quite a lot of functionality added:

On top of that, umbrella function for getting data has been added. Also, data format from Metobs has been changed to datetime to accomodate using diff() when building interpolation strategies.

I figure it is best to merge this back into main before developing further. Next step is to look at interpolation strategies and include mesan and strång functionality. Also, both unit and integration tests need to be added / updated.

Example to show one means by which I plan to include interpolation: Looking at nearby stations. When evaluating the snow depth around Kiruna, data is available starting from the late 19th century. Some stations lack data for extended periods in time, however. We can address the problem by looking at all stations around the city by running something like this code snippet:

import matplotlib.pyplot as plt
from smhi.smhi import SMHI

client = SMHI()
client.get_stations(3) # Causes error if not run. BUG!
client.find_stations_by_city(8,'Kiruna',20)

for nearby_station in client.nearby_stations:
    header, data = client.get_data(8,nearby_station[0])
    plt.plot(data['datetime'],data['Snödjup'])
plt.show()

snödjup_runt_kiruna

docNord commented 1 year ago

@mgcth could you help me resolve this?

mgcth commented 1 year ago

Is functionality done but missing tests?

docNord commented 1 year ago

I will fix it after work today. Functionality was done but I started over to fix black and lint and tests, require some hours. You will have a PR to review tonight 😊

On Tue, 24 Jan 2023, 23:09 Mladen Gibanica, @.***> wrote:

Is functionality done but missing tests?

— Reply to this email directly, view it on GitHub https://github.com/Ingenjorsarbete-For-Klimatet/ifk-smhi/pull/35#issuecomment-1402757403, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE7PLB5CVZQGRD4TUMRZVPLWUBHJTANCNFSM6AAAAAASMD2QRI . You are receiving this because you modified the open/close state.Message ID: @.***>

docNord commented 1 year ago

Alright, the smhi feature works for Metobs. Should we incorporate fast access to the other subcategories as well?

Example:

from smhi.smhi import SMHI
s = SMHI()
s.find_stations_by_city(8,'Kiruna')
s.nearby_stations
h1,d1 = s.get_data(8,180960)
_ ,d2 = s.get_data(8,180960,interpolate=40)

Kiruna_snödjup

docNord commented 1 year ago

Metfcts would also benefit from functionality similar to the Metobs wrapper functions just developed, and I am sure Mesan and Strang as well - it will be very succint if you can ask our SMHI client for data from a named place and get it automatically.

How do we go about setting this up? @mgcth you have a parallell branch to align data output formats from the different modules, once that is in place it should be doable to align with minor adjustments.

mgcth commented 1 year ago

Metfcts would also benefit from functionality similar to the Metobs wrapper functions just developed, and I am sure Mesan and Strang as well - it will be very succint if you can ask our SMHI client for data from a named place and get it automatically.

How do we go about setting this up? @mgcth you have a parallell branch to align data output formats from the different modules, once that is in place it should be doable to align with minor adjustments.

Create an issue about it, and we can talk more about design there.

docNord commented 1 year ago

What status do we have here @mgcth ?

mgcth commented 1 year ago

Haven't had the time to look at it. Will try this afternoon.

mgcth commented 1 year ago

Maybe you want to fix the latin1 and utf-8 encoding of the fixture file before merging. Also, do the docs need updating?

docNord commented 1 year ago

I don't know about the fixture file - why should we change it? The docs probably need editing, I could have a look quickly if you like.

mgcth commented 1 year ago

Because latin1 encoding fails on your win machine... Let's fix it later.

docNord commented 1 year ago

Right - but why can't we just use utf8 as the encoding in the test? That works on my windows machine, is it causing problems on your side?

docNord commented 1 year ago

Also, looking at the docs: I'm thinking adding an example page for smhi would be good? Otherwise, I don't know where to add the stuff we've been working at here...

mgcth commented 1 year ago

@docNord, can you try running tests from the latest commit on your local computer?

docNord commented 1 year ago

The new variant works on my Windows machine too. Great work Mladen! With that, ok to merge? Will create a new PR, hopefully tomorrow, for adding place search to the other protocols.