issue_comments

Unit Test Results

6 files  ±0           6 suites  ±0   59m 58s :stopwatch: ±0s 16 218 tests ±0  14 482 :heavy_check_mark: ±0  1 735 :zzz: ±0  1 :x: ±0  90 504 runs  ±0  82 323 :heavy_check_mark: ±0  8 175 :zzz: ±0  6 :x: ±0 

For more details on these failures, see this check.

Results for commit 41099669. ± Comparison against base commit 41099669.

:recycle: This comment has been updated with latest results.

That's unrelated, see #5654

Ah, I see. Thanks for merging!

That's unrelated, see #5654

Looks great. I'll merge when tests pass.

Thanks @tomchor. I see this is your first PR here. Welcome to xarray!

Thanks!

I think you spoke too soon though haha. One of the tests is failing and I don't really know why! I never touched the test_chunk() function...:

=========================== short test summary info ============================ FAILED xarray/tests/test_sparse.py::test_chunk - TypeError: operand type(s) a... = 1 failed, 13681 passed, 1110 skipped, 225 xfailed, 67 xpassed, 212 warnings in 445.02s (0:07:25) = **PNC:/usr/share/miniconda/envs/xarray-tests/lib/python3.7/site-packages/PseudoNetCDF/pncwarn.py:24:UserWarning: pyproj could not be found, so IO/API coordinates cannot be converted to lat/lon; to fix, install pyproj or basemap (e.g., `pip install pyproj)` Error: Process completed with exit code 1.

Looks great. I'll merge when tests pass.

Thanks @tomchor. I see this is your first PR here. Welcome to xarray!

Hello @tomchor! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-08-17 01:40:59 UTC

@tomchor thanks. Yes all you need to do is define a test function that checks something and pytest will run it.

you can run it following the instructions here: https://xarray.pydata.org/en/stable/contributing.html#running-the-test-suite

(something like this)

pytest xarray/tests/test_utils.py -k test_latex_name_isnt_split

Thanks! This makes it much easier to test everything locally. I test the new test locally and it should pass now. I also saw that test_chunk() isn't passing and I don't know why. Also I can't run that test locally for some reason.

@tomchor thanks. Yes all you need to do is define a test function that checks something and pytest will run it.

you can run it following the instructions here: https://xarray.pydata.org/en/stable/contributing.html#running-the-test-suite

(something like this) pytest xarray/tests/test_utils.py -k test_latex_name_isnt_split

I defined a function that tests that a long latex sequence stays intact after going through label_from_attrs(), but I'm not sure if that's the same as adding the test. It seems from the Pytest docs that you don't need to call the function explicitly; just define it. In which case that do the trick.

@dcherian could you please confirm?

Also the tests weren't passing before I added this last commit, and I have no idea why.

I wonder if it will look best if we avoid splitting inside the latex code? name, extra and units can have latex as well so I think we need to handle the entire name+extra+units string somehow.

I was playing around a little with regex but I didn't get that far:

```python import textwrap import re

name = r"$Ra_s = \mathrm{mean}(\epsilon_k) / \mu M^2_\infty$" extra = "Longwinded comments about something confusing. " units = r"$[N / m]$" label_raw = name + extra + units label_notex = re.sub("\$(.+?)\$", "X ", label_raw, flags=re.DOTALL) label_notex_wrap = textwrap.wrap(label_notex, 30) label_tex_wrap = ... # regex to add back the latex strings.
```

There might very well be a simpler way of doing this.

I couldn't figure out any way to avoid splitting LaTeX code. I suspect that the way to do it might be kind of complex (but I'm happy if someone can prove me wrong). We can also increase the number of columns for the latex option even more to maintain simplicity...

I wonder if it will look best if we avoid splitting inside the latex code? name, extra and units can have latex as well so I think we need to handle the entire name+extra+units string somehow.

I was playing around a little with regex but I didn't get that far: ```python import textwrap import re

name = r"$Ra_s = \mathrm{mean}(\epsilon_k) / \mu M^2_\infty$" extra = "Longwinded comments about something confusing. " units = r"$[N / m]$" label_raw = name + extra + units label_notex = re.sub("\$(.+?)\$", "X ", label_raw, flags=re.DOTALL) label_notex_wrap = textwrap.wrap(label_notex, 30) label_tex_wrap = ... # regex to add back the latex strings.
``` There might very well be a simpler way of doing this.