Skip to content

Redefine field_plausible_during_life and field_plausible_temporal_after#454

Closed
MaximMoinat wants to merge 8 commits intodevelopfrom
MaximMoinat-patch-2
Closed

Redefine field_plausible_during_life and field_plausible_temporal_after#454
MaximMoinat wants to merge 8 commits intodevelopfrom
MaximMoinat-patch-2

Conversation

@MaximMoinat
Copy link
Collaborator

@MaximMoinat MaximMoinat commented May 20, 2023

Redefine plausibleDuringLife, to be in line with the name; checking for events that happen before birth OR after death, instead of only checking for events after death.

Removes the date of birth check from field_plausible_temporal_after.sql.

Lastly, the date of birth derivation logic has changed. It is taken either from birth_datetime if available, otherwise calculated from year/month/day_of_birth, taking 1st if month/day not available (instead of using July 1st). See #445.

@MaximMoinat MaximMoinat changed the base branch from main to develop May 20, 2023 05:30
@codecov
Copy link

codecov bot commented May 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2883027) 86.42% compared to head (ca7247a) 86.42%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #454   +/-   ##
========================================
  Coverage    86.42%   86.42%           
========================================
  Files           16       16           
  Lines          884      884           
========================================
  Hits           764      764           
  Misses         120      120           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MaximMoinat MaximMoinat changed the title Refactor field_plausible_during_life Redefine field_plausible_during_life and field_plausible_temporal_after Dec 14, 2023
@MaximMoinat
Copy link
Collaborator Author

MaximMoinat commented Dec 14, 2023

To the reviewer(s): this check changes the meaning of both checks, making results harder to compare between DQD versions. It might be better to create completely new checks (e.g. into three new checks: plausibleAfterBirth, plausibleEndAfterStart, plausibleBeforeDeath)

Copy link
Collaborator

@dimshitc dimshitc left a comment

Choose a reason for hiding this comment

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

All good, but we agreed to split it into 3 checks. So, please let me know when this is done, and I'll approve the PR.

@MaximMoinat
Copy link
Collaborator Author

Closing this PR, reimplemented as three new checks in #516

@MaximMoinat MaximMoinat deleted the MaximMoinat-patch-2 branch February 13, 2024 12:43
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