kensho-technologies / graphql-compiler

Turn complex GraphQL queries into optimized database queries.
Apache License 2.0
553 stars 50 forks source link

Use inclusive language [removing sanity check] #984

Closed chewselene closed 3 years ago

chewselene commented 3 years ago

Remove ableist language.

codecov[bot] commented 3 years ago

Codecov Report

Merging #984 (b5a1dbe) into main (4fd8dda) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #984   +/-   ##
=======================================
  Coverage   94.33%   94.33%           
=======================================
  Files         113      113           
  Lines        8987     8987           
=======================================
  Hits         8478     8478           
  Misses        509      509           
Impacted Files Coverage Δ
graphql_compiler/compiler/compiler_frontend.py 98.57% <ø> (ø)
graphql_compiler/compiler/cypher_query.py 100.00% <ø> (ø)
graphql_compiler/cost_estimation/statistics.py 93.05% <ø> (ø)
...aphql_compiler/schema_generation/graphql_schema.py 95.89% <ø> (ø)
...schema_generation/orientdb/schema_graph_builder.py 95.87% <ø> (ø)
...hema_generation/sqlalchemy/sqlalchemy_reflector.py 97.95% <ø> (ø)
...l_compiler/compiler/ir_lowering_cypher/__init__.py 100.00% <100.00%> (ø)
..._compiler/compiler/ir_lowering_gremlin/__init__.py 100.00% <100.00%> (ø)
...ql_compiler/compiler/ir_lowering_match/__init__.py 100.00% <100.00%> (ø)
...ql_compiler/compiler/ir_self_consistency_checks.py 97.05% <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 4fd8dda...b5a1dbe. Read the comment docs.

chewselene commented 3 years ago

Instead of self_consistency_check and self_consistency_checks, what about assert and assertions? They describe what's actually happening, are unambiguous to the reader, and are much easier to read, type, and say.

I originally had this as validate/validation! gonna loop in @obi1kenobi who said: would it be okay to use self-consistency check instead of validate when possible for this? there’s a subtle distinction, but compiler IR can be locally valid (all real blocks with valid arguments etc.) but self-inconsistent (invariants that span multiple blocks are not upheld) so for example, if there’s a Traverse without a matching Backtrack, or a Traverse to a location that is never marked with MarkLocation

I think assert/assertion likely has a similar issue to validate/validation, but Branen also pointed out that all these functions do raise assertion errors so maybe it is appropriate?