Fix Context.childContext(null) to default to empty context_values object #282
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it.
Added a broken test showing that when you call Context.childContext(null), it results in null context_values instead of the intended default empty obect ({}).
Codecov Report
@@ Coverage Diff @@
## master #282 +/- ##
=======================================
Coverage 86.22% 86.22%
=======================================
Files 50 50
Lines 4400 4400
Branches 1234 1236 +2
=======================================
Hits 3794 3794
Misses 319 319
Partials 287 287
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
mgramigna
reviewed
Oct 24, 2022
mgramigna
approved these changes
Oct 26, 2022
birick1
pushed a commit
that referenced
this pull request
Jan 15, 2023
…ect (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest
mgramigna
pushed a commit
that referenced
this pull request
Jan 16, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
cmoesel
added a commit
that referenced
this pull request
Jan 16, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
mgramigna
pushed a commit
that referenced
this pull request
Jan 16, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
mgramigna
pushed a commit
that referenced
this pull request
Jan 16, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
mgramigna
pushed a commit
that referenced
this pull request
Mar 27, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
mgramigna
pushed a commit
that referenced
this pull request
Mar 27, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
mgramigna
pushed a commit
that referenced
this pull request
Mar 27, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
mgramigna
pushed a commit
that referenced
this pull request
Mar 28, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
mgramigna
pushed a commit
that referenced
this pull request
Mar 29, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
mgramigna
pushed a commit
that referenced
this pull request
Apr 21, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
cmoesel
added a commit
that referenced
this pull request
May 4, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
cmoesel
added a commit
that referenced
this pull request
May 4, 2023
The main motivation of this pull request is to introduce a targeted performance improvement to list operations. The list and query distinct operations are non-performant on large lists which do occur in real-world use. These performance issues are resolved by using normalized immutable objects (with the Immutable.js library) as keys (or hashes) optimized for value comparisons. In implementing the performance improvements, an issue was identified with the Uncertainty type where point equality with other types was not commutative. This could produce unexpected behavior based on the order in which Uncertain points were compared. This is corrected and tests added (thanks @hossenlopp / @cmoesel for constructor.name improvement suggestions). All Commits: * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add cross-env dependency for cross-platform test support; Update package-lock to v2 (npm 7+) * Fix a number of Typescript errors for test files using @ts-nocheck; Fix broken tests * Update Uncertainty to account for different types/constructors for comparison; Update Uncertainty.equals to account for point behavior; Add Uncertainty tests * Focused performance improvements for lists (doDistinct, doExcept, doIntersect) and queries (toDistinctList) * Resolve ESLint issues in Windows; Resolve ESLint and Prettier issues; * Correct wrong ESLint update for eslintrc.js * Update cql4browsers.js * Update ESLint config to use ecmaVersion 8 to align with es2017 update * Add ESLint curly rule for if {}; Fix nonconformant if statements; Remove redundant IMemoizeOptions; Remove unnecessary instanceof Uncertainty * Resolve cql4broswers, eslint, and prettier issues * Using 'has' has better performance * Change type checks in uncertainty; Add test to distinguish possible solutions * Update test comment * Change normalization to use fixed names where possible * Change normalization to use constructor (instead of constructor.name) * Add a _typeHierarchy check in normalization for model objects * Reword comment. * Rename memoizer.ts; Simplify; Update descriptions. * Fix Context.childContext(null) to default to empty context_values object (#282) * Fix Context.childContext to default to {} on undefined AND null Fixes #277 * Context type updates & UnfilteredContext bug fix Updated Context constructor to indicate that a Library can be passed in as the parent. Updated PatientContext and UnfilteredContext library property to indicate it is, in fact, a Library. In the process of doing this, discovered code that was looking for expression identifiers at the root of the Library object instead of Library.expressions. Oops! Fixed it. * Upgrade gradle wrapper * npm audit fix + npm install --save-dev mocha@latest * v2.4.3 (#284) * Re-up to package lock v2 after merge from master * Update build:test-data, build:spec-test-data, and watch:test-data scripts to run in Windows env too * Resolve npm security vulnerability via audit * Return doExcept and doIntersect to original implementations; Remove toKvp and toKvpParams in immutableUtil * Revert doUnion * Update cql4browsers post doUnion reversion * Prettier spacing * Update var names to reflect change from kvp to just keys * Correct build/watch scripts for windows/mac * Replace the toDistinctList implementation with in query.ts with list.ts implementation; Rename * Add .version/.display for normalized key for codes * Update test name * Add description for immutableUtil * Revert .eslintrc.js to .eslintrc.json removing the linebreak rule but keeping curly rule * Browserify, lint, prettier update post changes * Post Rebase Corrections * Matt/Chris Review updates * Resolve commit issues Co-authored-by: Chris Moesel <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some cases, a Context's
childContextfunction may be called with anullargument. In #277, this was happening as part of a sort comparator in a query. WhenchildContextis called withnull, thecontext_valuesshould default to{}, but instead they were being set tonull. This caused a crash when other code tried to access thosecontext_values.Bonus content:
To reproduce w/ unit tests:
git checkout fix-null-context-values).git reset --hard 4a4856dto go back to the commit w/ a broken test demonstrating the problem.npm testand note the failure (AssertionError: expected null to equal Object {}).git pullto get back up to date (w/ the fix).npm testagain and see the glorious green font of success.To reproduce manually:
Fixes #277
CDS Connect and Bonnie are the main users of this repository.
It is strongly recommended to include a person from each of those projects as a reviewer.
Submitter:
npm run test:plusto run tests, lint, and prettier)cql4browsers.jsbuilt withnpm run build:browserifyif source changed.Reviewer:
Name: