Skip to content

Conversation

@mcollaudcoen
Copy link
Member

The folder contains now the input(s) and output(s) for at least one test for all functions. Is that ok for a first testing procedure until the creation of the doi ?

mcollaudcoen and others added 20 commits September 17, 2020 12:14
Great. Merging this for now. We can certainly adapt the naming scheme if needed. I already removed the `D` for prewhite. And for the high level function, I followed your lead. It's best to avoid capital letters in Python, but other than that, I can match what you feel is best.
…ation (test1), with 4 meteorological seasons (test2) and with 12 months (test3)
@mcollaudcoen mcollaudcoen requested review from abigmo and fpavogt October 1, 2020 16:12
Copy link
Member

@fpavogt fpavogt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made a series of comments directly in each files.

One thing I noticed is the use of both , and ; for separating numbers. It would be good to stick with one throughout, ideally ,.
Edit: It also appears that in some files, missing values are simply not written, whereas in others they are explicitely tagged as NaN. It would be great to specify NaN everywhere.

One question about std_normal_var(): the first variable (s) is supposed to be a float. But s_test() returns it as an int. Which is the correct format ?

Some old files should also be removed from the repository as part of this PR (e.g. using git rm ... instead of git add ...):
Nb_tie_test1_out.txt
Nb_tie_test2_out.txt

I would also suggest renaming these files as follow (to stick to English spelling):
MK_tempAggr_testX_out_defaut.csv to MK_tempAggr_testX_out_default.csv

README.md Outdated

**compute_MK_stat:**
1. `[compute_MK_stat_test1_out1,compute_MK_stat_test1_out2,compute_MK_stat_test1_out3,compute_MK_stat_test1_out4]=compute_MK_stat(compute_MK_stat_test1_in1,compute_MK_stat_test1_in3,compute_MK_stat_test1_in3);`
1. `[compute_MK_stat_test2_out1,compute_MK_stat_test2_out2,compute_MK_stat_test2_out3,compute_MK_stat_test2_out4]=compute_MK_stat(compute_MK_stat_test2_in1,compute_MK_stat_test2_in3,compute_MK_stat_test2_in3,'alpha_MK',90,'alpha_CL',95);`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. `[compute_MK_stat_test2_out1,compute_MK_stat_test2_out2,compute_MK_stat_test2_out3,compute_MK_stat_test2_out4]=compute_MK_stat(compute_MK_stat_test2_in1,compute_MK_stat_test2_in3,compute_MK_stat_test2_in3,'alpha_MK',90,'alpha_CL',95);`
2. `[compute_MK_stat_test2_out1,compute_MK_stat_test2_out2,compute_MK_stat_test2_out3,compute_MK_stat_test2_out4]=compute_MK_stat(compute_MK_stat_test2_in1,compute_MK_stat_test2_in3,compute_MK_stat_test2_in3,'alpha_MK',90,'alpha_CL',95);`

@@ -0,0 +1,7306 @@
year,month,day,hour,minute,second,scat
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
year,month,day,hour,minute,second,scat

For consistency with prewhite_test1_in.csv, this file should have no header line. Or it needs to be added there.

mcollaudcoen and others added 7 commits October 22, 2020 18:43
@mcollaudcoen
Copy link
Member Author

I finish to change the test data files: there is one line with variables names (and sometimes nothing) at the beginning of each file.
Missing values were also changed to NaN in every files.

After your tests, I (or you) can merge it into the master branch

Copy link
Member

@fpavogt fpavogt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for me to merge this after adding the extra digit in compute_MK_stat_test1_out3.csv. All tests succeed in Python. 🎉

@@ -1 +1,2 @@
vari
24135957902.3305
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
24135957902.3305
24135957902.33049

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get one extra digit with Python. Adding it would make my life a lot easier ... Pretty please ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants