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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to loading docs to cover absence of 'or' for constraints #4321

Merged

Conversation

wjbenfold
Copy link
Contributor

馃殌 Pull Request

Description

Added documentation to Iris Constrained Loading docs to highlight that Constraints don't support 娄 (and explain why)

Whilst addressing a recent support request it seemed that there should be an ability to logically "or" Constraints as well as to "and" them. The reasons for this weren't immediately obvious, so documenting it seems useful for the future. I've done so.

I'd note, I'm not 100% convinced that this is an improvement to the quality of the docs, as I can see the argument that in catering for an edge case of misunderstanding the docs may become less easily understood by those without this misunderstanding (particularly as loading is likely to be one of the first bits of documentation a new user reads). Happy to discuss the merits of this and/or axe this PR on that basis.


Consult Iris pull request check list

@jamesp
Copy link
Member

jamesp commented Sep 16, 2021

Nice addition @wjbenfold. This should probably close #4091 as not going to be implemented.

@jamesp jamesp linked an issue Sep 16, 2021 that may be closed by this pull request
@pp-mo
Copy link
Member

pp-mo commented Sep 16, 2021

@jamesp This should probably close #4091 as not going to be implemented.

Hint: worth trying -- you can add it to "Linked issues" on the rhs ...

Copy link
Contributor

@stephenworsley stephenworsley left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple suggestions.

docs/src/userguide/loading_iris_cubes.rst Outdated Show resolved Hide resolved
docs/src/userguide/loading_iris_cubes.rst Outdated Show resolved Hide resolved
docs/src/userguide/loading_iris_cubes.rst Outdated Show resolved Hide resolved
lib/iris/_constraints.py Outdated Show resolved Hide resolved
Copy link
Contributor

@stephenworsley stephenworsley left a comment

Choose a reason for hiding this comment

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

Looks good, just a tiny comment and I think this is good to go.

lib/iris/_constraints.py Show resolved Hide resolved
@wjbenfold wjbenfold force-pushed the wjbenfold_load_docs_constraint_or branch from 7293dc5 to 6ab437a Compare September 20, 2021 10:49
@wjbenfold
Copy link
Contributor Author

wjbenfold commented Sep 20, 2021

Rebased onto main

Copy link
Contributor

@stephenworsley stephenworsley left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@stephenworsley stephenworsley merged commit ec442e1 into SciTools:main Sep 20, 2021
@wjbenfold wjbenfold deleted the wjbenfold_load_docs_constraint_or branch September 20, 2021 16:13
tkknight added a commit to tkknight/iris that referenced this pull request Sep 22, 2021
* main: (94 commits)
  added support for make html-noapi and html-quick for the docs build (SciTools#4333)
  Refactor some netcdf save code (SciTools#4301)
  [pre-commit.ci] pre-commit autoupdate (SciTools#4329)
  Update to loading docs to cover absence of 'or' for constraints (SciTools#4321)
  update latest.rst.template (SciTools#4323)
  stable cartopy feature download (SciTools#4328)
  Skip TestConstrainedLoad if data missing (SciTools#4319)
  Add 'Good First Issue' label to reasons an issue doesn't go stale (SciTools#4317)
  Gallery: simplify quiver example (SciTools#4120)
  Improve styling in a minor way in docs (SciTools#4314)
  bump version (SciTools#4310)
  Made clear we only test on Linux. (SciTools#4309)
  Updated environment lockfiles (SciTools#4308)
  Include Discussions in Getting Involved. (SciTools#4307)
  Fixed text to show as link. (SciTools#4305)
  [pre-commit.ci] pre-commit autoupdate (SciTools#4299)
  Updated environment lockfiles (SciTools#4298)
  cartopy feature download (SciTools#4304)
  Mesh Loading (AVD-1813) (SciTools#4262)
  reset whatsnew latest (SciTools#4288)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement OR for Iris ConstraintCombinations
5 participants