modified open_mdsdataset to add extra_variables

[cspencerjones]

Fixes #185 . The user will be able to specify extra variables that are non-standard and don't exist in available_diagnostics.log.

[codecov-commenter]

Codecov Report

Merging #205 into master will decrease coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #205      +/-   ##
==========================================
- Coverage   88.07%   88.00%   -0.08%     
==========================================
  Files          12       12              
  Lines        1644     1742      +98     
  Branches      337      358      +21     
==========================================
+ Hits         1448     1533      +85     
- Misses        158      168      +10     
- Partials       38       41       +3     

Impacted Files	Coverage Δ
xmitgcm/mds_store.py	92.35% <100.00%> (-0.37%)	⬇️
xmitgcm/llcreader/stores.py	75.51% <0.00%> (-13.97%)	⬇️
xmitgcm/llcreader/known_models.py	79.48% <0.00%> (+1.10%)	⬆️
xmitgcm/llcreader/llcmodel.py	82.49% <0.00%> (+2.81%)	⬆️

---

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 1fb9cc2...e195785. Read the comment docs.

[cspencerjones]

Presumably I should write a test in order to keep up code coverage? Any other feedback?

[rabernat]

Looks great Spencer! Thanks for getting this started!

Your implementation seems perfect. W just need a test. My recommendation would be to copy one of existing example files to a new name and then use extra_ variables to provide the metadata. An example of a test that uses file copying is here:

xmitgcm/xmitgcm/test/test_mds_store.py

Lines 493 to 504 in 3ab1fdb

	def test_drc_length(all_mds_datadirs):
	"""Test that open_mdsdataset is adding an extra level to drC if it has length nr"""
	dirname, expected = all_mds_datadirs
	# Only older versions of the gcm have len(drC) = nr, so force len(drC) = nr for the test
	copyfile(os.path.join(dirname, 'DRF.data'),
	os.path.join(dirname, 'DRC.data'))
	copyfile(os.path.join(dirname, 'DRF.meta'),
	os.path.join(dirname, 'DRC.meta'))
	ds = xmitgcm.open_mdsdataset(
	dirname, iters=None, read_grid=True,
	geometry=expected['geometry'])
	assert len(ds.drC) == (len(ds.drF)+1)

[rabernat]

An example is great, but you should also formally state exactly which fields are required and what they should contain.

[AaronDavidSchneider]

Hi, I tried to add a test and extend the docstring for this PR cspencerjones#2.
I really like the idea of this PR. Could be very useful.

[AaronDavidSchneider]

Hi @rabernat and @raphaeldussin, could you have a look on this PR?

[rabernat]

Hi @rabernat and @raphaeldussin, could you have a look on this PR?

Thanks for your work Aaron. I generally prefer to review PRs after the tests are passing. Do you need some help figuring out the test failures?

[AaronDavidSchneider]

Hi @rabernat and @raphaeldussin, could you have a look on this PR?

Thanks for your work Aaron. I generally prefer to review PRs after the tests are passing. Do you need some help figuring out the test failures?

Hi @rabernat, thanks for your swift reply! Yes indeed I could need some help here, since the tests that fail seem to not fail locally on my machine and the assertion errors look totally unrelated to this PR.
Is it possible that this PR needs to be rebased?

[rabernat]

Is it possible that this PR needs to be rebased?

Unlikely the cause. The code being tested in the workflow is exactly the same commit (f26ef84) that you have locally. The one thing that might be different is the environment. What version of xarray do you have installed locally?

[AaronDavidSchneider]

Yes indeed. That makes sense. My local environment uses xarray==0.16.2.

[rabernat]

Ok, so the new code and tests look great. No suggestions! Thanks so much for the work @cspencerjones and @AaronDavidSchneider.

I am fine with merging this. However, our CI has uncovered a bug in swap_dims that needs to be fixed asap!