iris

iris copied to clipboard

For #3741. This code now is able to read the sample LBC files with a 360 day calendar. I am now using this pull request to run the standard tests and seek further advice on adding extra tests (there don't seem to be any LBC files at the moment in the Iris data collection) and on whether it is desirable to check the values of LBTIM in some way.

Jun 17 '20 18:06 nhsavage

ok - so half of the Travis CI tests failed, I'd like some help in interpreting those failures too, as on a quick scan, I can't see what the problem is

Jun 17 '20 19:06 nhsavage

I now see what the error is iris.tests.unit.fileformats.ff.test_FF2PP.Test__adjust_field_for_lbc is no longer raising an error. I had not looked at this issue so now I will go back and look at that

Jun 18 '20 06:06 nhsavage

ok - so half of the Travis CI tests failed, I'd like some help in interpreting those failures too, as on a quick scan, I can't see what the problem is

• Within this PR: click on Details for the Travis CI section of the PR checks
• From the list of build jobs in the body of the Travis build's page: click on any of these marked as failing (you can click anywhere on the desired line)
• Within the job log section of the build job's page: click the circle in the top right of the log, which takes you to the bottom of the log
• Scroll up a little, and you will find details/traceback for any test failures directly below the list of test successes

Jun 18 '20 08:06 trexfeathers

ok, so all that is now failing is black. I am confused - I thought that Iris automatically applied black when I commit? How can I fix this?

Jun 18 '20 13:06 nhsavage

so now I have all the tests passing the next question is what additional tests should I be adding to the code? At the moment, the tests for LBCs are all based on mock. Should we add some real data as well? Also I would like to see tests for: 360 day calendar and variable resolution LBCs in addition to the fairly standard version we have here.

Jun 18 '20 21:06 nhsavage

Hi @nhsavage, thanks for your work on this; it's good to see the open source model working as intended.

Since you are within the Met Office, it would be good to discuss your outstanding questions in detail at our next surgery on Thursday 2nd July. Does that work for you? We wouldn't plan to merge this between now and then anyway (due to some other significant changes currently in the works).

Jun 19 '20 12:06 trexfeathers

Sorry Martin,

that doesn’t work for me because the next one is when I am taking annual leave (I was meant to be walking in the Highlands, instead I’ll be lucky to make it to Dartmoor…)

Nick

From: Martin Yeo [email protected] Sent: 19 June 2020 13:38 To: SciTools/iris [email protected] Cc: Savage, Nicholas [email protected]; Mention [email protected] Subject: Re: [SciTools/iris] Remove bugs when reading 360 day calendar LBCs (#3743)

Hi @nhsavagehttps://github.com/nhsavage, thanks for your work on this; it's good to see the open source model working as intended.

Since you are within the Met Office, it would be good to discuss your outstanding questions in detail at our next surgery on Thursday 2nd July. Does that work for you? We wouldn't plan to merge this between now and then anyway (due to some other significant changes currently in the works).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/SciTools/iris/pull/3743#issuecomment-646612764, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABPAHXQSH5Z7JPX35NXFEZDRXNLZ7ANCNFSM4OA32OLQ.

Jun 19 '20 13:06 nhsavage

@SciTools/peloton any chance of progressing this @nhsavage ?

Mar 29 '23 09:03 pp-mo

@SciTools/peloton any chance of progressing this @nhsavage ?

Probably not realistically. At the time of the last activity, I hadn't use mocking in tests before and so was stuck. I don't have any time specifically to work on this (it is useful but not essential to our work to do this and we have managed for 3 years without it. Is there any demand for this feature/fix from anyone except me? If not I suggest we close this

Mar 29 '23 11:03 nhsavage

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:exclamation: No coverage uploaded for pull request base (main@6d24b30). Click here to learn what that means.

@@           Coverage Diff           @@
##             main    #3743   +/-   ##
=======================================
  Coverage        ?   89.27%           
=======================================
  Files           ?       88           
  Lines           ?    22257           
  Branches        ?     4867           
=======================================
  Hits            ?    19870           
  Misses          ?     1641           
  Partials        ?      746           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Mar 29 '23 11:03 codecov[bot]

All committers have signed the CLA.

Apr 08 '24 16:04 CLAassistant