Implement from_namelist method

[malmans2]

• Closes Basic error handling for namelist imports #47
• Closes Deprecate ldbletanh #38
• Tests added
• Passes pre-commit run --all-files
• New functions/methods are listed in api.rst
• Changes are documented in whats-new.rst
• Project, label, and assignee tabs are populated

[codecov]

Codecov Report

Merging #43 (2165ab7) into main (006b9f3) will decrease coverage by 0.78%.
The diff coverage is 93.45%.

@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
- Coverage   94.85%   94.07%   -0.79%     
==========================================
  Files           5        5              
  Lines         214      287      +73     
==========================================
+ Hits          203      270      +67     
- Misses         11       17       +6     

Flag	Coverage Δ
unittests	94.07% <93.45%> (-0.79%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pydomcfg/domzgr/zgr.py	100.00% <ø> (ø)
pydomcfg/accessor.py	93.24% <90.00%> (-6.76%)	⬇️
pydomcfg/domzgr/zco.py	91.83% <93.75%> (-0.55%)	⬇️
pydomcfg/utils.py	97.22% <97.56%> (-0.15%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 006b9f3...2165ab7. Read the comment docs.

[malmans2]

OK, it's coming together. Looks like this right now:

ds_bathy = Bathymetry(1.0e3, 1.2e3, 1, 1).flat(5.0e3)
ds_bathy.domcfg.nml_ref_path = "path/to/namelist_ref"
ds_zgr = ds_bathy.domcfg.from_namelist("path/to/namelist_cfg")

[malmans2]

@jdha CI log shows thattest_zco_orca2 with the argument from_namelist=True is failing.
The error is:

TypeError: Value does not match expected type for nitbkg

We are testing using this namelist_ref and this namelist_cfg.

[jdha]

@jdha CI log shows thattest_zco_orca2 with the argument from_namelist=True is failing.
The error is:

TypeError: Value does not match expected type for nitbkg

We are testing using this namelist_ref and this namelist_cfg.

OK - thanks @malmans2 for pointing out the namelists. Looks like the domain cfg namelists have some legacy 3.6 bits in there (most of which a redundant: e.g. nambdy). I'll scape all the possible permutations of namelist variables from the Fortran and update the checking asap).

[malmans2]

OK! A couple of things that we might consider:

1. Should we just get rid of useless blocks before chaining? Looks like we only need just a few of them and it would make things easier I think.
2. Should we be less strict on namelist format and raise warnings rather than errors? I.e., we rely on our Zgr classes for errors and warnings will help debugging as they're raised before entering the Zgr class.

[jdha]

OK! A couple of things that we might consider:

1. Should we just get rid of useless blocks before chaining? Looks like we only need just a few of them and it would make things easier I think.
2. Should we be less strict on namelist format and raise warnings rather than errors? I.e., we rely on our Zgr classes for errors and warnings will help debugging as they're raised before entering the Zgr class.

re: 1. I was already looking at that - seems crazy to read in the whole namelist if you only need a few namblocks. I'll double check with the fortran to see which namblocks are active so we can refine the code to read just those. I think I'll code the exceptions anyway as I'll probably end up using the code for the NEMO namelists too.

re: 2. could do - but if the type or length is not correct, it ain't going run, no? I'm not convinced - but I'll take another look.

[malmans2]

@jdha I've changed it a bit and applied checks only to variables that we actually use. I'm going to push the changes to this PR. Let me know your thoughts (we can always revert back to your version). I haven't tested it too much though. Could you please test the new function _check_namelist_entries with your namelists that you know should pass/fail? In pydomcfg right now we are pretty much only looking at ln variables...

[jdha]

@malmans2 sorry didn't have time to finish the namelist stuff off today - I'll review it tomorrow morning.

[malmans2]

I think we can now move _check_namelist_entries in Zgr, so we run the exact same checks to the namelists and the arguments of our classes. (I think we just have to make a small change to support optional arguments).

This is why I'm using isinstance(value, int) which is a less strict check compared to type(value) == int.
For example, isinstance(True, int) is True which is OK for pydomcfg although NEMO would probably complain.
But I think it makes sense to use isinstance as we are not creating namelists to feed into NEMO.

[jdha]

@malmans2 I guess this is where nml_meld will diverge then as 1 != .true. in NEMO speak. But I think that's fine.

[malmans2]

Right, if eventually pydomcfg will make use of nml_meld, I think we just need an extra argument list isinstance=True for pydomcfg but False by default.

[jdha]

Just had a quick play and it seems to read a cut-down NEMO namelist_cfg ok [after removing exotics such as bn_]. However, should you have a namelist with:
chained['nn_it000']=[1, 1]
which is highly unlikely! The error thrown is:
Value does not match expected types for 'nn_it000'
rather than the preferred:
Mismatch in number of values provided for 'nn_it000'
Also in the namelist_cfg for DOMAINcfg Tools I noticed you'll also have to cater for:
jp* => int
pp* => real
cp* => str
but from memory ldbletanh is redundant now, so no need to include ld* => bool ?

[malmans2]

Just had a quick play and it seems to read a cut-down NEMO namelist_cfg ok [after removing exotics such as bn_]. However, should you have a namelist with:
chained['nn_it000']=[1, 1]
which is highly unlikely! The error thrown is:
Value does not match expected types for 'nn_it000'
rather than the preferred:
Mismatch in number of values provided for 'nn_it000'

OK! I think we should still raise a TypeError (a list is passed where we expect an int), but let's change the error message in that case. If we expect a list and a list with wrong size is passed we raise a ValueError.

Also in the namelist_cfg for DOMAINcfg Tools I noticed you'll also have to cater for:
jp* => int
pp* => real
cp* => str
but from memory ldbletanh is redundant now, so no need to include ld* => bool ?

Let's add those as well, should be easy! Even if we deprecate ldbletanh, that flag will be used by from_namelist.

[malmans2]

@pyNEMO/pydomcfg This is ready for review!

[jdha]

Looks good. Well all the bits I understand at least! Just trying to get my head around wraps. But I'll have a play with some examples this afternoon.

[malmans2]

Thanks!
@wraps is just a convenience function for decorators/wrappers. Basically it makes sure signatures and properties of decorated functions don't change (e.g., __name__, __doc__, __annotations__, ...).

[jdha]

Thanks!
@wraps is just a convenience function for decorators/wrappers. Basically it makes sure signatures and properties of decorated functions don't change (e.g., __name__, __doc__, __annotations__, ...).

Yeah. I read as much - just need to try it out. I guess I was a little puzzled as to when you wouldn't want that functionality? Is there a case when that kind of inheritance is undesirable? Probably a silly question ...

[malmans2]

Thanks!
@wraps is just a convenience function for decorators/wrappers. Basically it makes sure signatures and properties of decorated functions don't change (e.g., __name__, __doc__, __annotations__, ...).

Yeah. I read as much - just need to try it out. I guess I was a little puzzled as to when you wouldn't want that functionality? Is there a case when that kind of inheritance is undesirable? Probably a silly question ...

In our case we use the decorator just to run some extra checks before entering the function. But I guess for decorators that actually change the behavior of the decorated functions, @wraps is probably not the best choice.