ncasuk / amf-check-writer

Library to write AMF compliance checks
BSD 3-Clause "New" or "Revised" License
0 stars 4 forks source link

Fix filename check when running amf-checker? #62

Closed gapintheclouds closed 2 years ago

gapintheclouds commented 2 years ago

When running amf-checker using the following filename it doesn't accept the filename:

$ TEST_FILE=ncas-anemometer-1_ral_29001225_mean-winds_corrected_v0.1.nc
$ amf-checker --yaml-dir $DATA_DIR/$VERSION/checks $TEST_FILE --version $VERSION
WARNING: Filename 'ncas-anemometer-1_ral_29001225_mean-winds_corrected_v0.1.nc' does not match expected format '<instrument_name>_<platform_name>_<YYYYMM><DD><HH><mm><SS>_<data_product>[_<option1>_<option2>]_v<version>.nc'

It works if the file is renamed to ncas-anemometer-1_ral_29001225_mean-winds_corrected_monkey_v0.1.nc. It only allows filenames with both <option1> and <option2> not just one option.

Is this correct behaviour?

agstephens commented 2 years ago

@barbarabrooks please can you advise us on this issue. The bit of code that tests the file name is:

https://github.com/ncasuk/amf-check-writer/blob/master/amf_check_writer/amf_checker.py#L17-L31

It assumes that you either get "" or just "" in the file name. However, should it allow any of:

Let us know :-)

barbarabrooks commented 2 years ago

OK

so line 20

.

should be

-.

There is a hyphen separating day and hour.

After there is always a _

so you can have

_v __v ___v ____v __v ___v __v Is that what you were meaning? Cheers Barb On 28/09/2021 17:46, Ag Stephens wrote: > > @barbarabrooks please can you > advise us on this issue. The bit of code that tests the file name is: > > https://github.com/ncasuk/amf-check-writer/blob/master/amf_check_writer/amf_checker.py#L17-L31 > > > It assumes that you either get "///" or just "/" in the file name. > However, should it allow any of: > > * |_| > * |__| > * |___| > > Let us know :-) > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > , > or unsubscribe > . > Triage notifications on the go with GitHub Mobile for iOS > > or Android > . > > -- ------------------------------------------ NCAS Fairbairn House 71 - 75 Clarendon Road Leeds LS2 9PH T: 0113 343 3574 M: 07738275748 W: https://amof.ac.uk/
agstephens commented 2 years ago

Thanks @barbarabrooks, that's clearer. I just reviewed the _vocabularies.xlsx file at:

https://docs.google.com/spreadsheets/d/13GkA7BTKaqzfsSBiPy2LAK-2hg0rC2p3hHJMS19POas/edit#gid=1377025008

It looks like we don't have exact/exhaustive options for each <optionN> component so I don't think we should try to do vocab-checking against them. We should just make sure the structure is correct.

@gapintheclouds : please see @barbarabrooks's response above. Please implement that as a regex - and write a few more unit tests to check that it works okay. Thanks

gapintheclouds commented 2 years ago

Thanks.

@barbarabrooks : For the date/time string. I guess we should allow any of the following:

\\ (a year always needs an accompanying month) \\\

\\\
-\ \\\
-\\ \\\
-\\\

Is that correct? Or should the time always be \\\?

agstephens commented 2 years ago

@gapintheclouds: as part of this work, please do the following:

  1. Move tests.py from amf_check_writer/ to tests/test_check_writer.py - don't worry about getting these running
  2. Create a new tests/test_filename.py - with lots of quick tests to check regex for filename is doing the right thing.
agstephens commented 2 years ago

Fix now implemented.