Skip to content

CDDSO-326 fix calendar initialisation in qc validators#161

Merged
piotr-florek-mohc merged 3 commits intov2.5_releasefrom
CDDSO-326-fix-calendar-initialisation-in-QC-validators
Aug 4, 2023
Merged

CDDSO-326 fix calendar initialisation in qc validators#161
piotr-florek-mohc merged 3 commits intov2.5_releasefrom
CDDSO-326-fix-calendar-initialisation-in-QC-validators

Conversation

@piotr-florek-mohc
Copy link
Contributor

just a simple fix plus some unit tests

Copy link
Collaborator

@matthew-mizielinski matthew-mizielinski left a comment

Choose a reason for hiding this comment

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

LGTM and tests pass

@piotr-florek-mohc piotr-florek-mohc merged commit a2ae008 into v2.5_release Aug 4, 2023
@mo-jareddrayton
Copy link
Collaborator

One thing that we might need to be careful of is the state of the isodatetime Calendar singleton. As it stands it will be changed to "gregorian" when calling .date_validator but not switched back. This could cause problems if any validation code that needs the "360_day" calendar is added or moved to after this change of calendar.

@piotr-florek-mohc piotr-florek-mohc changed the title Cddso 326 fix calendar initialisation in qc validators CDDSO-326 fix calendar initialisation in qc validators Aug 4, 2023
piotr-florek-mohc added a commit that referenced this pull request Aug 4, 2023
* CDDSO-326 fixed validator and added unit tests

* CDDSO-326 more unit testing

* CDDSO-326 copyright update
@piotr-florek-mohc
Copy link
Contributor Author

One thing that we might need to be careful of is the state of the isodatetime Calendar singleton. As it stands it will be changed to "gregorian" when calling .date_validator but not switched back. This could cause problems if any validation code that needs the "360_day" calendar is added or moved to after this change of calendar.

Good catch, I forgot that the default() method returns a singleton. Will fix that asap.

@mo-jareddrayton
Copy link
Collaborator

Normally quite handy to set and forget but in cases like this it's kind of awkward to handle! I'd probably opt to use cf_time or datetime for this validator rather than set and then reset the Calendar in isodatetime.

@piotr-florek-mohc piotr-florek-mohc deleted the CDDSO-326-fix-calendar-initialisation-in-QC-validators branch August 25, 2023 15:52
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.

3 participants