Skip to content

Validate option series contract fields - #2

Merged
JustinMShea merged 12 commits into
masterfrom
fix/option-series-validation
Sep 5, 2026
Merged

JustinMShea merged 12 commits into
masterfrom
fix/option-series-validation

Conversation

@JustinMShea

Copy link
Copy Markdown
Owner

Adds validation for option-series expiration dates, option rights, strikes, and trading dates. Exact expiration dates are inferred from option identifiers, inconsistent explicit fields produce clear conditions, and focused tinytest coverage is added without changing the exported API.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds internal validation and parsing for option-series contract fields so option_series() can infer exact expiration dates (including day), normalize option rights, validate strikes, and enforce trading date constraints while maintaining backward compatibility via warnings when explicit fields disagree with the identifier.

Changes:

  • Introduces internal helpers to parse option suffixes and validate/normalize expires, callput, strike, and first_traded.
  • Updates option_series() to construct and validate identifiers consistently, emitting targeted warnings for mismatches.
  • Adds focused tinytest coverage for exact-date parsing, normalization, warnings, and error cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
R/option_series_validation.R Adds internal parsing/validation helpers for option-series expiration dates, rights, and strikes.
R/instrument.R Switches option_series() to use the new helpers for construction/validation and to infer exact expiration dates.
inst/tinytest/test_option_series.R Adds tests covering exact-date parsing, OSI-style strikes, normalization, warnings, and invalid inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread R/option_series_validation.R Outdated
JustinMShea and others added 2 commits July 13, 2026 19:47
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread R/option_series_validation.R
Comment thread R/option_series_validation.R
JustinMShea and others added 3 commits July 13, 2026 20:08
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread R/option_series_validation.R
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread R/option_series_validation.R Outdated
JustinMShea and others added 2 commits July 13, 2026 21:09
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@JustinMShea

Copy link
Copy Markdown
Owner Author

@copilot Review this PR for backward-compatibility risks, edge cases, and unintended API changes. Pay special attention to constructor behavior, parsing/normalization logic, date handling, warnings vs errors, POSIXct/POSIXlt inputs, vectorized inputs, and compatibility with existing FinancialInstrument conventions. Check whether the tests adequately cover both new functionality and legacy behavior. Do not suggest stylistic refactors unless they materially improve correctness or safety.

Verify OSI-style option symbols, strike parsing, call/put normalization, explicit-vs-inferred field mismatches, and POSIXct/POSIXlt handling.

Co-authored-by: JustinMShea <10689703+JustinMShea@users.noreply.github.com>

Copilot AI commented Sep 5, 2026

Copy link
Copy Markdown

@copilot Review this PR for backward-compatibility risks, edge cases, and unintended API changes. Pay special attention to constructor behavior, parsing/normalization logic, date handling, warnings vs...

I have reviewed the PR across all requested areas in commit c9f40ba:

  • Constructor behavior & field defaults: Updated option_series() field construction mode so omitting callput or passing callput = NULL gives clear error conditions or resolves inferred values consistently.
  • Date handling (POSIXct/POSIXlt): .option_series_date() formats POSIXt timestamps to %Y-%m-%d calendar dates in their native representation, preventing time-zone boundary shifts. first_traded is normalized and stored as formatted date strings alongside expires.
  • OSI symbols & strike parsing: Verified 8-digit fixed-width fractional and integer strikes in opt2 and opt4 formats.
  • Vectorized inputs & compatibility: Verified vectorized primary_id calls, enforcing that expires/first_traded remain NULL for multi-id invocations.
  • Test coverage: Extended inst/tinytest/test_option_series.R to cover OSI fractional strikes, field constructor errors, callput = NULL, normalized first_traded, and vectorized primary_id inputs.

…alidation

origin master m
A
A
A
A
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
B
A
B
B
B
B
B
B
A
A
A
A
A
A
A
A
B
B
B
B
B
B
B
@JustinMShea
JustinMShea merged commit fcb1da5 into master Sep 5, 2026
7 checks passed
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