Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change order of dims in example of make_xarray_grid #329

Merged
merged 2 commits into from Aug 28, 2021
Merged

Conversation

santisoler
Copy link
Member

@santisoler santisoler commented Aug 11, 2021

Change the order of the dimensions of DataArrays in the examples of
make_xarray_grid, since xarray v0.19.0 prints them in a different order.
Tell pytest to ignore the outputs of the print statements in the
make_xarray_grid examples while running doctests.

Reminders:

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst and the base __init__.py file for the package.
  • Write detailed docstrings for all functions/classes/methods. It often helps to design better code if you write the docstrings first.
  • If adding new functionality, add an example to the docstring, gallery, and/or tutorials.
  • Add your full name, affiliation, and ORCID (optional) to the AUTHORS.md file (if you haven't already) in case you'd like to be listed as an author on the Zenodo archive of the next release.

@santisoler
Copy link
Member Author

@leouieda The xarray version that is changing the default behavior is 0.19.0, while on Python3.6 conda installs xarray 0.18.2.
Now we have a single example that prints different outputs depending on the version you are running. How can we tackle this?

@santisoler
Copy link
Member Author

Should we tell pytest to skip running the print lines? https://docs.pytest.org/en/latest/how-to/doctest.html#skipping-tests

@leouieda
Copy link
Member

My recommendation here is to change the test. I’ve been bitten by tests relying on reprs and they inevitably break. This happens even with numpy arrays, which are relatively stable. So the best thing to do is to check attributes, types, or print formatted values instead of relying on the repr.

@santisoler
Copy link
Member Author

santisoler commented Aug 27, 2021

My recommendation here is to change the test. I’ve been bitten by tests relying on reprs and they inevitably break. This happens even with numpy arrays, which are relatively stable. So the best thing to do is to check attributes, types, or print formatted values instead of relying on the repr.

Ok. So the idea is to tell pytest to don't check the output of the print statement and possibly add a few other lines in the example showing the attributes of the generated output. Does it make sense?

@santisoler
Copy link
Member Author

I think this should be enough. The tests for the make_xarray_grid are complete enough, so I think asking pytest to ignore those print statements on the examples is a good compromise. Let me know what do you think, and feel free to merge if you like it.

@leouieda
Copy link
Member

My idea was more "don't use print in doctests" but this is better since the example is easier to read 👍

@leouieda leouieda merged commit b53ae75 into master Aug 28, 2021
@leouieda leouieda deleted the xarray-dims branch August 28, 2021 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants