Skip to content

Fix operator checks to follow other check_type logic.#85

Merged
lvrfrc87 merged 2 commits intodevelopfrom
fix_operator_checks
Sep 23, 2022
Merged

Fix operator checks to follow other check_type logic.#85
lvrfrc87 merged 2 commits intodevelopfrom
fix_operator_checks

Conversation

@pszulczewski
Copy link
Collaborator

@pszulczewski pszulczewski commented Sep 22, 2022 •
edited
Loading

This PR fixes operator check logic and returned results to make them the same as for other check types.
When the check passes then returned data result is empty, only when the check fails data result is returned to indicate why the check failed.
This is the main behavior for other checks and I think operator check should follow the same behavior.

pszulczewski
pszulczewski commented Sep 22, 2022
View reviewed changes
jdiff/check_types.py
params_value = params.get("params", {}).get("operator_data")

if not params_key or not params_value:
if not params_key or params_value is None:
Copy link
Collaborator Author

@pszulczewski pszulczewski Sep 22, 2022

Choose a reason for hiding this comment

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

This was making all-same check to fail here when False was passed as value.

lvrfrc87
lvrfrc87 reviewed Sep 22, 2022
View reviewed changes
Copy link
Collaborator

@lvrfrc87 lvrfrc87 left a comment

Choose a reason for hiding this comment

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

I believe we need to review this along with @chadell. This approach is the negative of what implemented today. I understand that the idea behind that is to unify the way the test return the bool result but I believe it would be confusing adopt this strategy.

lvrfrc87
lvrfrc87 previously approved these changes Sep 22, 2022
View reviewed changes
Copy link
Collaborator

@lvrfrc87 lvrfrc87 left a comment

Choose a reason for hiding this comment

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

As discussed, this is now good to go! Thanks for helping me to see this from a different point of view @pszulczewski !

@lvrfrc87
Copy link
Collaborator

lvrfrc87 commented Sep 22, 2022

@pszulczewski would you mind to update the CHANGELOG and relevant bits in poetry?

@lvrfrc87 lvrfrc87 mentioned this pull request Sep 22, 2022
Closed
lvrfrc87
lvrfrc87 approved these changes Sep 23, 2022
View reviewed changes
Copy link
Collaborator

@lvrfrc87 lvrfrc87 left a comment

Choose a reason for hiding this comment

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

LGTM

@lvrfrc87 lvrfrc87 merged commit 7d47d7c into develop Sep 23, 2022
@lvrfrc87 lvrfrc87 deleted the fix_operator_checks branch September 23, 2022 08:47
lvrfrc87 added a commit that referenced this pull request Sep 23, 2022
@lvrfrc87 @itdependsnetworks
@pszulczewski @scetron
Release 0.0.2 (#88)
a539dbe 
* Update readme to start with use cases (#84)

* Update readme to start with use cases

* Apply suggestions from code review

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Update README.md

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Doc update (#87)

* Fix operator checks to follow other check_type logic. (#85)

* Fix operator checks to follow other check_type logic.

* Add new release 0.0.2

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>
lvrfrc87 added a commit that referenced this pull request Sep 27, 2022
@lvrfrc87 @itdependsnetworks
@pszulczewski @scetron
Implement ge and le operator type (#89)
afa094d 
* Release 0.0.2  (#88)

* Update readme to start with use cases (#84)

* Update readme to start with use cases

* Apply suggestions from code review

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Update README.md

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Doc update (#87)

* Fix operator checks to follow other check_type logic. (#85)

* Fix operator checks to follow other check_type logic.

* Add new release 0.0.2

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* add operator ge and le

* fix validate tests

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>
@lvrfrc87 lvrfrc87 mentioned this pull request Apr 20, 2023
Merged
lvrfrc87 added a commit that referenced this pull request Apr 20, 2023
@lvrfrc87 @itdependsnetworks
@pszulczewski @scetron @dependabot
Ver. 0.0.3 release. (#99)
7a5ae75 
* Update readme to start with use cases (#84)

* Update readme to start with use cases

* Apply suggestions from code review

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Update README.md

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Doc update (#87)

* Fix operator checks to follow other check_type logic. (#85)

* Fix operator checks to follow other check_type logic.

* Add new release 0.0.2

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Implement `ge` and `le` operator type (#89)

* Release 0.0.2  (#88)

* Update readme to start with use cases (#84)

* Update readme to start with use cases

* Apply suggestions from code review

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Update README.md

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Doc update (#87)

* Fix operator checks to follow other check_type logic. (#85)

* Fix operator checks to follow other check_type logic.

* Add new release 0.0.2

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* add operator ge and le

* fix validate tests

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Fix parameter_match check, to support non-normalized data (#90)

* Fix parameter_match check, to support non-normalized data

* Add test

* Update tests.

* remove test print

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Update mypy

* Bugfix to data normalization

* Fix ref_key parsing

* Update docs

* Issue 92 (#98)

* Base-line tests for future refactor.

* Refactor to simplify code.

* Implement issue #92

* Bump certifi from 2022.9.24 to 2022.12.7 (#95)

Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.24 to 2022.12.7.
- [Release notes](https://github.com/certifi/python-certifi/releases)
- [Commits](certifi/python-certifi@2022.09.24...2022.12.07)

---
updated-dependencies:
- dependency-name: certifi
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
lvrfrc87 added a commit that referenced this pull request May 12, 2023
@pszulczewski @itdependsnetworks
@scetron @lvrfrc87 @dependabot @Kircheneer
Release v0.0.4 (#102)
1879e6d 
* Update readme to start with use cases (#84)

* Update readme to start with use cases

* Apply suggestions from code review

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Update README.md

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Doc update (#87)

* Fix operator checks to follow other check_type logic. (#85)

* Fix operator checks to follow other check_type logic.

* Add new release 0.0.2

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Implement `ge` and `le` operator type (#89)

* Release 0.0.2  (#88)

* Update readme to start with use cases (#84)

* Update readme to start with use cases

* Apply suggestions from code review

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Update README.md

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Doc update (#87)

* Fix operator checks to follow other check_type logic. (#85)

* Fix operator checks to follow other check_type logic.

* Add new release 0.0.2

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* add operator ge and le

* fix validate tests

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Fix parameter_match check, to support non-normalized data (#90)

* Fix parameter_match check, to support non-normalized data

* Add test

* Update tests.

* remove test print

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Update mypy

* Bugfix to data normalization

* Fix ref_key parsing

* Update docs

* Issue 92 (#98)

* Base-line tests for future refactor.

* Refactor to simplify code.

* Implement issue #92

* Bump certifi from 2022.9.24 to 2022.12.7 (#95)

Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.24 to 2022.12.7.
- [Release notes](https://github.com/certifi/python-certifi/releases)
- [Commits](certifi/python-certifi@2022.09.24...2022.12.07)

---
updated-dependencies:
- dependency-name: certifi
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update version and changelog

* update poetry lock

* Update changelog

* Issue 94 & 91 (#100)

* Baseline unittests

* Fix issue #94

* Remove test print

* Fix issue #91

* Relax deepdiff dependency.

* Update pyproject.toml

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>

* Update poetry.lock

* Relase v0.0.4

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>
Co-authored-by: Federico87 <15066806+lvrfrc87@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Network to Code <federico.olivieri@networktocode.com>
Co-authored-by: Leo Kirchner <leo.kirchner98@gmail.com>
Co-authored-by: Leo Kirchner <Kircheneer@users.noreply.github.com>
lvrfrc87 added a commit that referenced this pull request May 12, 2023
@pszulczewski @itdependsnetworks
@scetron @lvrfrc87 @dependabot @Kircheneer
Release v0.0.4 (#106)
4a887ff 
* Update readme to start with use cases (#84)

* Update readme to start with use cases

* Apply suggestions from code review

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Update README.md

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Doc update (#87)

* Fix operator checks to follow other check_type logic. (#85)

* Fix operator checks to follow other check_type logic.

* Add new release 0.0.2

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Implement `ge` and `le` operator type (#89)

* Release 0.0.2  (#88)

* Update readme to start with use cases (#84)

* Update readme to start with use cases

* Apply suggestions from code review

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Update README.md

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>

* Doc update (#87)

* Fix operator checks to follow other check_type logic. (#85)

* Fix operator checks to follow other check_type logic.

* Add new release 0.0.2

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* add operator ge and le

* fix validate tests

Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Fix parameter_match check, to support non-normalized data (#90)

* Fix parameter_match check, to support non-normalized data

* Add test

* Update tests.

* remove test print

Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>

* Update mypy

* Bugfix to data normalization

* Fix ref_key parsing

* Update docs

* Issue 92 (#98)

* Base-line tests for future refactor.

* Refactor to simplify code.

* Implement issue #92

* Bump certifi from 2022.9.24 to 2022.12.7 (#95)

Bumps [certifi](https://github.com/certifi/python-certifi) from 2022.9.24 to 2022.12.7.
- [Release notes](https://github.com/certifi/python-certifi/releases)
- [Commits](certifi/python-certifi@2022.09.24...2022.12.07)

---
updated-dependencies:
- dependency-name: certifi
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update version and changelog

* update poetry lock

* Update changelog

* Issue 94 & 91 (#100)

* Baseline unittests

* Fix issue #94

* Remove test print

* Fix issue #91

* Relax deepdiff dependency.

* Update pyproject.toml

Co-authored-by: Patryk Szulczewski <patryk@szulczewski.eu>

* Update poetry.lock

* Relase v0.0.4

* Release v0.0.4 (#105)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Christian Adell <christian.adell@networktocode.com>
Co-authored-by: Ken Celenza <ken@celenza.org>
Co-authored-by: Stephen Corry <stephen.corry@networktocode.com>
Co-authored-by: Patryk Szulczewski <patryk.szulczewski@networktocode.com>
Co-authored-by: Federico87 <15066806+lvrfrc87@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Network to Code <federico.olivieri@networktocode.com>
Co-authored-by: Leo Kirchner <leo.kirchner98@gmail.com>
Co-authored-by: Leo Kirchner <Kircheneer@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lvrfrc87 lvrfrc87 lvrfrc87 approved these changes

@chadell chadell Awaiting requested review from chadell

@scetron scetron Awaiting requested review from scetron scetron is a code owner

@jeffkala jeffkala Awaiting requested review from jeffkala jeffkala is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pszulczewski @lvrfrc87