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

Correct and improve dev-guide section on fixing graphics-tests. #3683

Merged
merged 8 commits into from
Mar 18, 2020

Conversation

pp-mo
Copy link
Member

@pp-mo pp-mo commented Mar 10, 2020

Hoping this is worth it, and that this actually is an improvement ..

I think this is needed, as I had to work out the fix process for myself, yet again, in course of #3682.

@pp-mo pp-mo requested review from lbdreyer and trexfeathers March 10, 2020 21:56
Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As well as the one change I have referenced for line 116, there are several changes to suggest that involve code you have not changed:

References to version numbers that date the file

At present (Iris version 1.10), such tests include the testing for modules

From Iris v1.11 onward, we want to support testing Iris against multiple

Paragraph still implies that this section only covers 1 of the BRIEF points, but it now covers all 3

If you notice that a graphics test in the Iris testing suite has failed

graphics_test.rst doesn't appear in docs/iris/src/developers_guide/index.rst

… so the page can't be accessed by navigation
https://github.com/SciTools/iris/blob/656a2b1ea97ce33f533a41de806da03342d711a2/docs/iris/src/developers_guide/index.rst

@pp-mo
Copy link
Member Author

pp-mo commented Mar 18, 2020

Hi @trexfeathers Given your above comments, it seems it needs a slightly more holistic content review, i.e. what this is, whether it is up-to-date + says the right things.
I'll take another look.

@pp-mo
Copy link
Member Author

pp-mo commented Mar 18, 2020

Hi @trexfeathers thanks for your attention.
Following your comments above, I've now reviewed the whole page and made quite a few changes.

Your point about this page not appearing on a TOC I think is not a particular problem..
The 'graphics_test' page is linked from a "graphics tests" section in the tests.rst overview page, as a "for-more-detail" reference. I think that in itself is ok -- it doesn't matter that it does not appear in an index ?
I check that you can still find it in searches.

Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there @pp-mo, thanks for persevering!

Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been through and made suggestions to create a consistent language around the 'acceptable' results (there were previously 4 different ways of saying this), including a clearer title for the section on adding new ones.

@trexfeathers
Copy link
Contributor

@pp-mo I believe if you rebase this onto master the tests will also pass again

@pp-mo pp-mo force-pushed the graphictest_docs branch from e20ea24 to 0c9c975 Compare March 18, 2020 17:04
@pp-mo
Copy link
Member Author

pp-mo commented Mar 18, 2020

Rebased ... 😎

@trexfeathers trexfeathers merged commit c5ecefb into SciTools:master Mar 18, 2020
trexfeathers added a commit that referenced this pull request Mar 18, 2020
* Fixed tests since Numpy 1.18 deprecation of non-int num arguments for linspace. (#3655)

* remove redundant tests (#3650)

* Remove obsolete test. (#3662)

* Remove grib-specific test. (#3663)

* Removed ununused skipIf. (#3632)

* Remove test_grib_save_rules.py which has been moved to iris-grib (#3666)

* 2v4 mergeback picks (#3668)

* Stop PPDataProxy accessing the file when no data is needed. (#3659)

* Add 2.4 whatsnew into full whatsnew list.

Co-authored-by: Martin Yeo <[email protected]>

* Remove uri callback test which is moved to iris-grib (#3665)

* Remove test_grib2 integration tests (#3664)

* Remove test_grib_save.py (#3669)

* Remove cube iter (#3656)

* Fixed asv project name to 'scitools-iris'. (#3660)

* Removed grib-specific test to iris-grib. (#3671)

* Removed iris.tests.integration.test_grib_load and related CML files. (#3670)

* Remove TestGribMessage (#3672)

* Switched use of datetime.weekday() to datetime.dayofwk. (#3687)

* New image hashes for mpl 3x2 (#3682)

* New image hash for iris.test.test_plot.TestSymbols.test_cloud_cover with matplotlib 3.2.0.

* Further images changes for mpl3x2.

* Yet more updated image results.

* Correct and improve dev-guide section on fixing graphics-tests. (#3683)

* Correct and improve dev-guide section on fixing graphics-tests.

* Review changes + general rethink.

* Reduce duplication between 'graphics-tests' and general 'tests' page.

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

Co-authored-by: Martin Yeo <[email protected]>

Co-authored-by: Martin Yeo <[email protected]>
Co-authored-by: Bill Little <[email protected]>
Co-authored-by: lbdreyer <[email protected]>
Co-authored-by: Jon Seddon <[email protected]>
@pp-mo
Copy link
Member Author

pp-mo commented Mar 18, 2020

Pheee - ew
Thanks @trexfeathers 😅

stephenworsley added a commit to stephenworsley/iris that referenced this pull request Jun 8, 2020
… default_units_patch

* 'default_units' of https://github.com/SciTools/iris:
  Unify saving behaviour of "unknown" and "no_unit" (SciTools#3711)
  Change default loading unit from "1" to "unknown" (correct branch) (SciTools#3709)
  Change default units to "unknown" for all DimensionalMetadata (SciTools#3713)
  Update docs CubeList.extract method (SciTools#3694)
  Correct and improve dev-guide section on fixing graphics-tests. (SciTools#3683)
  New image hashes for mpl 3x2 (SciTools#3682)
  Switched use of datetime.weekday() to datetime.dayofwk. (SciTools#3687)
  Remove TestGribMessage (SciTools#3672)
  Removed iris.tests.integration.test_grib_load and related CML files. (SciTools#3670)
  Removed grib-specific test to iris-grib. (SciTools#3671)
  Fixed asv project name to 'scitools-iris'. (SciTools#3660)
  Remove cube iter (SciTools#3656)
  Remove test_grib_save.py (SciTools#3669)
  Remove test_grib2 integration tests (SciTools#3664)
  Remove uri callback test which is moved to iris-grib (SciTools#3665)
  2v4 mergeback picks (SciTools#3668)
  Remove test_grib_save_rules.py which has been moved to iris-grib (SciTools#3666)
  Removed ununused skipIf. (SciTools#3632)
  Remove grib-specific test. (SciTools#3663)
  Remove obsolete test. (SciTools#3662)
@pp-mo pp-mo deleted the graphictest_docs branch June 11, 2020 09:39
tkknight pushed a commit to tkknight/iris that referenced this pull request Jun 29, 2020
…ools#3683)

* Correct and improve dev-guide section on fixing graphics-tests.

* Review changes + general rethink.

* Reduce duplication between 'graphics-tests' and general 'tests' page.

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

* Update docs/iris/src/developers_guide/graphics_tests.rst

Co-Authored-By: Martin Yeo <[email protected]>

Co-authored-by: Martin Yeo <[email protected]>
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.

2 participants