Facetgrid

[jukent]

Address #79

[codecov-commenter]

Codecov Report

Merging #83 into main will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #83   +/-   ##
=======================================
  Coverage   92.12%   92.12%           
=======================================
  Files           7        7           
  Lines         800      800           
=======================================
  Hits          737      737           
  Misses         63       63           

Flag	Coverage Δ
#unittests	92.12% <ø> (ø)

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

Impacted Files	Coverage Δ
cf_xarray/accessor.py	92.96% <ø> (ø)

---

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 235acac...b4a9a18. Read the comment docs.

[dcherian]

Julia, I think the issue may be the wrap_classes arg which is False by default. We want to set it to True for facetgrid plots.

cf-xarray/cf_xarray/accessor.py

Lines 530 to 531 in 19d80cf

	if wrap_classes and isinstance(result, _WRAPPED_CLASSES):
	result = _CFWrappedClass(result, accessor)

EDIT: I don't actually remember why this was added. Set to True by default and see if tests pass?

[codecov-io]

Codecov Report

Merging #83 (1833a22) into main (e11cf07) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 1833a22 differs from pull request most recent head 8e8b239. Consider uploading reports for the commit 8e8b239 to get more accurate results

@@            Coverage Diff             @@
##             main      #83      +/-   ##
==========================================
- Coverage   96.39%   96.38%   -0.01%     
==========================================
  Files          14       14              
  Lines        1635     1631       -4     
==========================================
- Hits         1576     1572       -4     
  Misses         59       59              

Flag	Coverage Δ
unittests	96.38% <ø> (-0.01%)	⬇️

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

Impacted Files	Coverage Δ
cf_xarray/accessor.py	95.17% <ø> (ø)
cf_xarray/tests/test_accessor.py	100.00% <ø> (ø)

---

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 e11cf07...8e8b239. Read the comment docs.

[dcherian]

assert isinstance(rv, xr.plot.FacetGrid)

This test should be changed; we aren't returning pure FacetGrid instances anymore.

[jukent]

assert isinstance(rv, xr.plot.FacetGrid)

This test should be changed; we aren't returning pure FacetGrid instances anymore.

That makes sense. I'm not 100% sure what the instances should be yet so I won't change it - at least until I have it working locally.

[dcherian]

We can just delete that line and make sure that plotting followed by map_dataarray works.

[jukent]

We can just delete that line and make sure that plotting followed by map_dataarray works.

There are 3 lines identical to that (on 298, 302, and 312). Should we delete each of these tests?

[dcherian]

yes unless you have ideas for something to test.