feature/tc_forecast_cxml by mmyrte · Pull Request #36 · CLIMADA-project/climada_petals

mmyrte · 2022-03-11T14:29:06Z

This is a work in progress for a Cyclone XML (cxml) reader for TC forecast products. I'm currently testing it with data from ECMWF, see https://confluence.ecmwf.int/display/TIGGE/Tools#Tools-tc, but due to the clear XML specs, it should work with all providers. General structure:

Transforms an xml file into csv using xsl, which is read into a pandas DF
Groups the pandas DF into chunks that constitute tracks.
Transforms each chunk into the accustomed xr.Dataset structure.

I've tried making it robust against duplicate/false labels in the input data, such as I encountered here (wrong basin designations), but cannot be sure yet.

Missing: Tests, gracious handling of missing dependencies, documentation.

mmyrte · 2022-04-28T08:48:39Z

Hi @aleeciu - I didn't get around to this recently; still working on a test. Would be happy if you could already have a first look, but totally cool if you don't run anything yet.

mmyrte · 2022-04-29T13:05:35Z

@aleeciu I've now committed unit tests, but here comes the hard part - I'm so out of touch with you guyses CI pipeline that I don't know how to add the lxml dependency or to graciously catch it if it's not already present. I guess this is also for @emanuel-schmid : I don't want to burden climada_petals with additional dependencies, since this feature only going to be used by very few people. How would you handle the situation?

aleeciu · 2022-04-29T13:10:51Z

since this feature only going to be used by very few people.

not sure about this, I think there could be some interest. Anyway it's on @emanuel-schmid to decide whether we are fine with adding lxml.

aleeciu · 2022-05-20T11:56:28Z

climada_petals/hazard/tc_tracks_forecast.py

 """Missing double and integers in ecCodes """

+CXML2CSV_XSL = Path(__file__).parent / "tc_tracks_foreast_cxml2csv.xsl"
+"""Path at which an xsl is found for transforming CXML to CSV format."""


do we want this file here or in the climada/data folder? @emanuel-schmid

Hi @emanuel-schmid, here is one issue.

We certainly don't want it here. climada_petals/hazard/data folder maybe. Or climada data api. Depends on what is that file. What's inside, who wrote it and how, will it ever change, can we have it as csv file instead?

So that file is required to translate the downloaded file from cxml into csv. It was written by @mmyrte, I'd say we can consider it as the standard CLIMADA file when working with these .cxml files. The module was written such that a user can potentially define locally his/her own conversion file and, if so, this standard file won't be used. On whether it can also be written as .csv it's for @mmyrte but I doubt.

@emanuel-schmid No, it's not a data file; it's XSL, so it contains transformation logic.

climada_petals/hazard/test/data/cxml_sample_transformation.xsl

mmyrte

I'm happy now. @aleeciu I don't know whether there are still things that need to be approved by @emanuel-schmid, but if it's really just the question of where in the file tree the xsl is positioned, that can easily be adjusted later on. I say we merge, unless there are new policies in place that I don't know of.

climada_petals/hazard/tc_tracks_forecast.py

climada_petals/hazard/test/data/cxml_sample_transformation.xsl

climada_petals/hazard/tc_tracks_forecast.py

aleeciu · 2022-05-23T13:59:45Z

@mmyrte thanks for this new feature. To me it looks quite fine. I will make a tutorial and test it with different providers. Then I think we can merge ? Do you anything else on your TODO list ?

mmyrte · 2022-05-23T14:25:57Z

To summarise our call:

You discuss with @emanuel-schmid whether you want to pin the lxml version or not (thereby removing the need for the string conversion or not).
You rename both or either of the cxml_sample_transformation.xsl files to something more appropriate, like cxml_ecmwf_transformation.xsl and cxml_60-72h_filter.xsl.
Plus you take care of a tutorial and such.

Cheers

Jan

aleeciu · 2022-05-23T14:29:36Z

To summarise our call: You discuss with @emanuel-schmid whether you want to pin the lxml version or not (thereby removing the need for the string conversion or not).
You rename both or either of the cxml_sample_transformation.xsl files to something more appropriate, like cxml_ecmwf_transformation.xsl and cxml_60-72h_filter.xsl.
Plus you take care of a tutorial and such.

Sounds like a plan

climada_petals/hazard/tc_tracks_forecast.py

…-project/climada_petals into feature/tc_forecast_cxml

climada_petals/hazard/tc_tracks_forecast.py

Also includes default type coercion for Path arguments to lxml's etree.parse; it can only handle pathlike args since v. 4.8.0

…-project/climada_petals into feature/tc_forecast_cxml

aleeciu · 2022-06-24T09:59:53Z

climada_petals/hazard/tc_tracks_forecast.py

 """Missing double and integers in ecCodes """

+CXML2CSV_XSL = Path(__file__).parent / "data/cxml_ecmwf_transformation.xsl"
+"""Xsl file for transforming CXML to CSV format."""


@emanuel-schmid I have now created a data folder in hazard and moved this transformation file therein. I am not sure whether this is the best place or if it should go somewhere else. I have tried to move it in my climada/data folder but then I get an error from jenkins that it does not find the file (I think I should have also changed something in the CONFIG file but I am not sure what).

aleeciu · 2022-06-24T10:13:14Z

Hi @emanuel-schmid,

to me this this now works (many thanks @mmyrte) and it's ready to be merged. Open issue is where to locate the data:

there is a file we need for the new feature to run. I have now moved it into a newly created folder hazard/data as you suggested above. But maybe we want them in climada/data ? I have tried doing so but then I get an error from jenkins saying it does not find the data. I think I should manipulate the CONFIG file if I want to fix that but I would not know exactly how.
I have started a tutorial and wanted my trial data to be in climada/data/demo. However I am not sure how to access them from the tutorial folder since when I call from climada.util.constants import DEMO_DIR it creates /climada_petals/doc/tutorial/data/demo instead of pointing to /climada_petals/data/demo. Again it's me not being able to properly handle the CONFIG file. We can however merge the branch without a tutorial and then add it later.

Thanks.

mmyrte · 2022-06-29T14:18:58Z

One last comment - I still don't think that application logic such as that contained in the xsl file should be in a "data" directory, since I find the name misleading. It's up to you guys though.

mmyrte added 3 commits March 11, 2022 15:15

fix linter errors

26d9c18

TCForecast: draft version of cxml reader

19e8626

Merge origin/develop into feature/tc_forecast_cxml

e82bf26

mmyrte mentioned this pull request Mar 11, 2022

Feature/tc forecast hdf5 #33

Merged

mmyrte added 2 commits March 11, 2022 17:32

fix coordinate unit issue

30ce567

TCForecast.read_cxml: add xsl as optional argument

a4f26ea

emanuel-schmid changed the title ~~Feature/tc forecast cxml~~ WIP: feature/tc_forecast_cxml Apr 7, 2022

mmyrte added 3 commits April 28, 2022 10:15

Add ability to pass custom xsl file

9b16301

More explicit about expected types

ddde6ae

Naming consistency

f5d8512

mmyrte requested a review from aleeciu April 28, 2022 08:29

mmyrte added 3 commits April 29, 2022 14:47

Better default env pressures

831f93c

More flexible default xsl

d44757e

Add unittests for cxml reader

f03f89c

mmyrte marked this pull request as ready for review April 29, 2022 12:54

fix typo

ea71861

aleeciu reviewed May 20, 2022

View reviewed changes

climada_petals/hazard/test/data/cxml_sample_transformation.xsl Show resolved Hide resolved

aleeciu and others added 5 commits May 23, 2022 14:12

switch test on

35764e5

convert pathlib.path to string

676b5eb

save only one basin value per track

5e00f1b

assign unique id

98dc7af

Nicer exception handling; handle linter complaints

3607f39

mmyrte commented May 23, 2022

View reviewed changes

mmyrte commented May 26, 2022

View reviewed changes

climada_petals/hazard/tc_tracks_forecast.py Outdated Show resolved Hide resolved

climada_petals/hazard/tc_tracks_forecast.py Outdated Show resolved Hide resolved

aleeciu added 3 commits May 26, 2022 12:01

add category and delta_time attrs

8a3b0f6

Merge branch 'feature/tc_forecast_cxml' of https://github.com/CLIMADA…

1ffc763

…-project/climada_petals into feature/tc_forecast_cxml

basin info for each track node

1b1f418

aleeciu reviewed May 26, 2022

View reviewed changes

climada_petals/hazard/tc_tracks_forecast.py Outdated Show resolved Hide resolved

aleeciu and others added 14 commits May 26, 2022 15:21

convert to knots and assign env pressure

7908c3b

add TODO

f2bae19

make it work with write_hdf5()

828163f

make it work with write_hdf5() (2)

a30164b

lxml as top-level import; import ordering

f572693

Also includes default type coercion for Path arguments to lxml's etree.parse; it can only handle pathlike args since v. 4.8.0

use lastClosedIsobar for env pressure

7a3230a

Merge branch 'feature/tc_forecast_cxml' of https://github.com/CLIMADA…

572608c

…-project/climada_petals into feature/tc_forecast_cxml

fix tests failure

e8bab13

fix test

f8ec30c

rename and move transformation files

52a5c28

add doc string

9807554

remove transform file

ec9d597

rename file

4c21854

remove unused import

37eeab7

aleeciu reviewed Jun 24, 2022

View reviewed changes

mmyrte closed this Jun 29, 2022

mmyrte reopened this Jun 29, 2022

emanuel-schmid added 3 commits August 12, 2022 17:45

Merge branch 'develop' into feature/tc_forecast_cxml

e304c03

cosmetics

4979462

cosmetics

a7b7fcf

emanuel-schmid merged commit f5a696e into develop Aug 15, 2022

emanuel-schmid deleted the feature/tc_forecast_cxml branch August 15, 2022 08:52

emanuel-schmid changed the title ~~WIP: feature/tc_forecast_cxml~~ feature/tc_forecast_cxml Aug 15, 2022

Conversation

mmyrte commented Mar 11, 2022

Uh oh!

mmyrte commented Apr 28, 2022

Uh oh!

mmyrte commented Apr 29, 2022

Uh oh!

aleeciu commented Apr 29, 2022

Uh oh!

aleeciu May 20, 2022

Choose a reason for hiding this comment

Uh oh!

aleeciu May 25, 2022

Choose a reason for hiding this comment

Uh oh!

emanuel-schmid May 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleeciu May 25, 2022

Choose a reason for hiding this comment

Uh oh!

mmyrte May 26, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mmyrte left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aleeciu commented May 23, 2022

Uh oh!

mmyrte commented May 23, 2022

Uh oh!

aleeciu commented May 23, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aleeciu Jun 24, 2022

Choose a reason for hiding this comment

Uh oh!

aleeciu commented Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmyrte commented Jun 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

emanuel-schmid May 25, 2022 •

edited

Loading

aleeciu commented Jun 24, 2022 •

edited

Loading