Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for storing snapshots with any time resolution in CSV files and File System Repository. #268

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cinar
Copy link
Owner

@cinar cinar commented Jan 18, 2025

Describe Request

This is to allow using different date time formats to store snapshots at various resolutions and no longer just days.

Added support for configuring the CSV file, and the File System Repository.

  • asset.Snapshot.Date no longer has a date format declared as a struct tag.
  • helper.NewCsv takes options to configure headers, logger, and the default date time format.
  • asset.NewFileSystemRepository also takes CSV options to configure the CSV files.

Example:

csv, err := helper.NewCsv[Row](
	helper.WithCsvDefaultDateTimeFormat[Row]("2006-01-02 15:04:05"),
)

Related #266

Change Type

New feature.

Summary by CodeRabbit

Based on the comprehensive summary, here are the release notes:

  • New Features

    • Enhanced CSV handling with more flexible configuration options
    • Introduced functional options for CSV processing
    • Added new configuration methods for CSV operations
  • Improvements

    • Simplified CSV reading functions
    • Removed hardcoded boolean parameters in CSV file reading
    • Updated default date format handling
  • Refactoring

    • Restructured CSV-related functions to use variadic options
    • Modified function signatures across multiple packages
    • Streamlined CSV data processing methods

Copy link

coderabbitai bot commented Jan 18, 2025

Walkthrough

This pull request introduces a comprehensive refactoring of CSV-related functionality across multiple packages. The primary changes involve modifying the ReadFromCsvFile function to use a more flexible, option-based approach instead of a boolean parameter. A new CsvOption type is introduced in the helper package, allowing for more granular configuration of CSV reading and writing operations. The changes span numerous test files and helper functions, removing the hasHeader boolean and replacing it with a more extensible configuration mechanism.

Changes

File Change Summary
helper/csv.go Added CsvOption type and related functional options like WithoutCsvHeader, WithCsvLogger, and WithCsvDefaultDateTimeFormat
asset/snapshot.go Removed format:"2006-01-02" tag from Date field
asset/file_system_repository.go Updated NewFileSystemRepository to accept variadic CSV options
Multiple test files Removed true argument from ReadFromCsvFile calls across various packages

Poem

🐰 A Rabbit's Ode to CSV Delight

With options new and headers light,
Our CSV dance takes playful flight,
No more boolean's rigid might,
Flexibility now shines so bright,
Coding magic at its height!

🎉 Hop, hop, hooray! 🎉

Finishing Touches

  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 87.09677% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.67%. Comparing base (8812266) to head (156b0ba).

Files with missing lines Patch % Lines
helper/csv.go 84.61% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #268      +/-   ##
==========================================
- Coverage   91.71%   91.67%   -0.04%     
==========================================
  Files         181      181              
  Lines        6540     6557      +17     
==========================================
+ Hits         5998     6011      +13     
- Misses        482      486       +4     
  Partials       60       60              

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

@cinar cinar changed the title Added support for configuring the CSV file. Adding support for storing snapshots with any time resolution in CSV files and File System Repository. Jan 18, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🔭 Outside diff range comments (1)
momentum/stochastic_oscillator_test.go (1)

Line range hint 1-1: Update documentation for new CSV configuration.

The PR removes the hasHeader parameter in favor of a more flexible configuration system. Please:

  1. Update the package documentation to explain the new CSV configuration options
  2. Add examples showing how to use the new CsvOption type
  3. Document any changes to default behavior
🧹 Nitpick comments (24)
helper/README.md (1)

Line range hint 1102-1195: LGTM! Well-designed options pattern implementation.

The implementation follows Go best practices by using the functional options pattern. The provided options cover essential configuration needs while maintaining extensibility.

Consider adding these options in the future:

  • WithCsvDelimiter for custom field separators
  • WithCsvQuote for custom quote characters
  • WithCsvComment for custom comment characters
trend/rma_test.go (1)

20-20: Consider adding examples and migration guide.

The changes to remove the hasHeader parameter in favor of configurable CSV options represent a good architectural improvement. Consider:

  1. Adding examples demonstrating the new CSV configuration options
  2. Providing a migration guide for users updating from the old API
  3. Including test cases that verify different CSV configurations (headers, date formats, etc.)
trend/typical_price_test.go (1)

22-22: Consider adding explicit CSV configuration for clarity.

Since this test reads multiple columns (High, Low, Close, TypicalPrice), it would benefit from explicit CSV configuration to document the expected format.

Example:

options := helper.NewCsvOption().WithHeaders(true)
input, err := helper.ReadFromCsvFile[Data]("testdata/typical_price.csv", options)
momentum/williams_r_test.go (1)

22-22: Maintain consistency with other test files.

For consistency with similar test files (like typical_price_test.go), consider adding explicit CSV configuration here as well.

Example:

options := helper.NewCsvOption().WithHeaders(true)
input, err := helper.ReadFromCsvFile[Data]("testdata/williams_r.csv", options)
trend/bop_test.go (1)

23-23: Document CSV file requirements.

Consider adding a comment describing the expected CSV structure to help maintain test data consistency.

Add a comment above the ReadFromCsvFile call:

+// testdata/bop.csv should contain columns: Open,High,Low,Close,Bop
 input, err := helper.ReadFromCsvFile[BopData]("testdata/bop.csv")
trend/mls_test.go (1)

22-22: Consider adding negative test cases.

With the new CSV configuration options, consider adding test cases that verify behavior with different CSV formats.

Add test cases for:

  • CSV files without headers
  • CSV files with different date time formats
  • CSV files with missing or malformed columns
trend/aroon_test.go (1)

21-24: Consider documenting CSV configuration changes.

The removal of the hasHeader parameter across multiple test files suggests a significant change in CSV handling. To maintain consistency and prevent future issues:

  1. Update documentation to reflect the new CSV configuration options
  2. Consider adding examples of different CSV configurations in the test suite
  3. Verify that all existing test data files match the new default configuration
strategy/trend/tsi_strategy_test.go (1)

Line range hint 1-1: Consider updating documentation and adding integration tests.

Since this PR introduces significant changes to CSV configuration:

  1. Consider adding integration tests that explicitly verify the new CSV configuration options
  2. Update documentation to reflect the removal of the boolean parameter and showcase the new configuration options
  3. Consider adding examples demonstrating different date format configurations
strategy/trend/alligator_strategy_test.go (1)

17-17: Consider adding explicit CSV configuration examples.

While removing the hasHeader parameter aligns with the new configurable approach, consider adding test cases that demonstrate the new CSV configuration capabilities, especially for different date time formats as mentioned in the PR objectives.

Example configuration:

+	csvOptions := helper.CsvOption{
+		HasHeader: true,
+		DateTimeFormat: "2006-01-02 15:04:05",
+	}
+
-	snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv")
+	snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv", csvOptions)

Also applies to: 22-22, 39-39

strategy/trend/golden_cross_strategy_test.go (1)

Line range hint 1-1: Overall Recommendations for Test Coverage

The changes consistently remove the hasHeader parameter from ReadFromCsvFile calls across all test files. However, to fully validate the PR objectives:

  1. Add test cases demonstrating the new CSV configuration capabilities, especially:
    • Different date time formats for snapshots
    • Explicit header configuration
    • Custom CSV options
  2. Update test data files to ensure they align with the new configuration approach
  3. Consider adding documentation in test files showing example usage of the new configuration options

These additions would help validate the new features and serve as usage examples for other developers.

strategy/volatility/bollinger_bands_strategy_test.go (1)

Line range hint 1-55: Consider creating a test helper for CSV operations.

Given that these CSV reading operations are repeated across multiple test files with the same configuration needs, consider creating a test helper function to encapsulate the common CSV reading logic with the appropriate options:

// test_helper.go
func readTestCsvFile[T any](t *testing.T, filename string) []T {
    csvOpts := helper.NewCsvOptions().
        WithHeaders(true).
        WithDefaultDateTimeFormat("2006-01-02") // adjust format as needed

    data, err := helper.ReadFromCsvFile[T](filename, csvOpts)
    if err != nil {
        t.Fatal(err)
    }
    return data
}

This would:

  1. Ensure consistent CSV handling across all tests
  2. Reduce code duplication
  3. Make it easier to modify CSV options in the future
  4. Improve test maintainability
strategy/volume/weighted_average_price_strategy_test.go (1)

Line range hint 1-52: Consider centralizing test CSV configuration.

Since the PR introduces configurable CSV options and these test files share common CSV reading patterns, consider:

  1. Creating a shared test helper function that provides default CSV configuration for tests
  2. Centralizing the test data format configuration to make it easier to maintain

Example approach:

// test_helper.go
func ReadTestSnapshot(t *testing.T, filename string) []asset.Snapshot {
    opts := []helper.CsvOption{
        helper.WithHeader(true),
        helper.WithDateFormat("2006-01-02"), // adjust format as needed
    }
    snapshots, err := helper.ReadFromCsvFile[asset.Snapshot](filename, opts...)
    if err != nil {
        t.Fatal(err)
    }
    return snapshots
}
strategy/or_strategy_test.go (2)

17-17: Consider adding explicit CSV configuration for date formats.

Since this test uses asset.Snapshot which has date fields, and compares with strategy results, consider adding explicit CSV configuration to ensure consistent date format handling across files.

+var csvOptions = helper.WithCsvDefaultDateTimeFormat("2006-01-02")
+
 func TestOrStrategy(t *testing.T) {
-	snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/repository/brk-b.csv")
+	snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/repository/brk-b.csv", csvOptions)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	results, err := helper.ReadFromCsvFile[strategy.Result]("testdata/or.csv")
+	results, err := helper.ReadFromCsvFile[strategy.Result]("testdata/or.csv", csvOptions)

Also applies to: 22-22


41-41: Apply consistent CSV configuration in report test.

For consistency, apply the same CSV configuration in the report test function.

 func TestOrStrategyReport(t *testing.T) {
-	snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/repository/brk-b.csv")
+	snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/repository/brk-b.csv", csvOptions)
trend/kdj_test.go (1)

24-24: Document the expected CSV format.

The change is consistent with other test files. Consider adding documentation about the expected CSV format in the test data files, especially since the header configuration has been removed.

Add a comment above the ReadFromCsvFile call:

+// The test data CSV file is expected to have headers matching the Data struct fields
 input, err := helper.ReadFromCsvFile[Data]("testdata/kdj.csv")
volatility/acceleration_bands_test.go (1)

Line range hint 1-1: Consider adding CSV configuration documentation and examples.

To support the new configurable CSV handling:

  1. Add examples in test files demonstrating different date format configurations
  2. Consider creating shared CSV configuration constants for commonly used formats
  3. Update documentation to explain the new configuration options
strategy/split_strategy_test.go (1)

Line range hint 16-40: Add test cases for different date formats.

Since the PR adds support for configurable date formats, consider adding test cases that verify this functionality with various date time formats (e.g., daily, hourly, minutes).

helper/report_test.go (1)

Line range hint 14-20: Document the precedence of date format configurations.

Since there are now multiple ways to specify the date format (struct tags and CSV options), consider documenting the precedence rules:

  • Which takes precedence: struct tags or CSV options?
  • How are conflicts resolved?
helper/csv_test.go (1)

396-419: Enhance test coverage for date format handling.

While the test validates basic functionality, consider adding tests for:

  1. Invalid date formats
  2. Different timezone handling
  3. Default behavior when no format is specified
  4. Various date-time format patterns
 func TestCsvWithDefaultDateFormat(t *testing.T) {
+    tests := []struct {
+        name     string
+        input    string
+        format   string
+        expected time.Time
+        wantErr  bool
+    }{
+        {
+            name:     "valid format",
+            input:    "2023-11-26 01:02:03",
+            format:   "2006-01-02 15:04:05",
+            expected: time.Date(2023, 11, 26, 1, 2, 3, 0, time.UTC),
+        },
+        {
+            name:    "invalid format",
+            input:   "2023-13-45 99:99:99",
+            format:  "2006-01-02 15:04:05",
+            wantErr: true,
+        },
+        // Add more test cases
+    }
+    // ... implement table-driven tests
 }
asset/file_system_repository.go (2)

24-25: Add documentation for the csvOptions field.

The new field needs documentation explaining:

  • Purpose of the options
  • Expected option types
  • Impact on CSV reading/writing
 // csvOptions are the CSV options used for reading and writing snapshots.
+// These options configure how CSV files are processed, including:
+// - Date-time format for parsing the Snapshot.Date field
+// - Header configuration for CSV files
+// - Additional CSV processing options
 csvOptions []helper.CsvOption[Snapshot]

29-34: Add validation and examples for CSV options.

The constructor should validate the options and provide usage examples in the documentation.

 // NewFileSystemRepository initializes a file system repository with
 // the given base directory and the CSV options.
+//
+// Example:
+//
+//  repo := NewFileSystemRepository("data",
+//    helper.WithCsvDefaultDateTimeFormat[Snapshot]("2006-01-02 15:04:05"),
+//    helper.WithoutCsvHeader[Snapshot](),
+//  )
 func NewFileSystemRepository(base string, csvOptions ...helper.CsvOption[Snapshot]) *FileSystemRepository {
+    // Validate base directory
+    if base == "" {
+        panic("base directory cannot be empty")
+    }
     return &FileSystemRepository{
         base:       base,
         csvOptions: csvOptions,
     }
 }
helper/csv.go (1)

55-77: Great use of the functional options pattern!

The implementation is clean, type-safe, and follows Go best practices. Consider adding validation for the date format string in WithCsvDefaultDateTimeFormat.

 func WithCsvDefaultDateTimeFormat[T any](format string) CsvOption[T] {
+	// Validate the format string by attempting to format a known date
+	testTime := time.Date(2006, 1, 2, 15, 4, 5, 0, time.UTC)
+	if _, err := testTime.Format(format); err != nil {
+		panic(fmt.Sprintf("invalid date format: %s", format))
+	}
 	return func(c *Csv[T]) {
 		c.defaultDateTimeFormat = format
 	}
 }
trend/kama_test.go (1)

Line range hint 1-1: Consider adding documentation for the new CSV configuration mechanism.

The PR introduces changes to support configurable CSV files, particularly for different date time formats. Consider adding:

  1. Documentation explaining the new CSV configuration options
  2. Example test cases demonstrating different date time format configurations
  3. Migration guide for updating existing code that used the hasHeader parameter
asset/file_system_repository_test.go (1)

72-72: Consider adding tests for the new CSV configuration options.

While the removal of the boolean parameter is correct, we should ensure there are tests covering the new CSV configuration options, particularly for different date time formats.

Would you like me to help generate additional test cases that cover various CSV configuration scenarios?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8812266 and 156b0ba.

📒 Files selected for processing (82)
  • asset/README.md (8 hunks)
  • asset/file_system_repository.go (3 hunks)
  • asset/file_system_repository_test.go (1 hunks)
  • asset/snapshot.go (1 hunks)
  • asset/snapshot_test.go (1 hunks)
  • helper/README.md (11 hunks)
  • helper/csv.go (5 hunks)
  • helper/csv_test.go (20 hunks)
  • helper/report_test.go (2 hunks)
  • momentum/awesome_oscillator_test.go (1 hunks)
  • momentum/chaikin_oscillator_test.go (1 hunks)
  • momentum/ichimoku_cloud_test.go (1 hunks)
  • momentum/ppo_test.go (1 hunks)
  • momentum/pvo_test.go (1 hunks)
  • momentum/qstick_test.go (1 hunks)
  • momentum/rsi_test.go (1 hunks)
  • momentum/stochastic_oscillator_test.go (1 hunks)
  • momentum/stochastic_rsi_test.go (1 hunks)
  • momentum/williams_r_test.go (1 hunks)
  • strategy/and_strategy_test.go (2 hunks)
  • strategy/buy_and_hold_strategy_test.go (2 hunks)
  • strategy/compound/macd_rsi_strategy_test.go (2 hunks)
  • strategy/decorator/inverse_strategy_test.go (2 hunks)
  • strategy/decorator/no_loss_strategy_test.go (2 hunks)
  • strategy/decorator/stop_loss_strategy_test.go (2 hunks)
  • strategy/majority_strategy_test.go (2 hunks)
  • strategy/momentum/awesome_oscillator_strategy_test.go (2 hunks)
  • strategy/momentum/rsi_strategy_test.go (2 hunks)
  • strategy/momentum/stochastic_rsi_strategy_test.go (2 hunks)
  • strategy/momentum/triple_rsi_strategy_test.go (2 hunks)
  • strategy/or_strategy_test.go (2 hunks)
  • strategy/split_strategy_test.go (2 hunks)
  • strategy/trend/alligator_strategy_test.go (2 hunks)
  • strategy/trend/apo_strategy_test.go (2 hunks)
  • strategy/trend/aroon_strategy_test.go (2 hunks)
  • strategy/trend/bop_strategy_test.go (2 hunks)
  • strategy/trend/cci_strategy_test.go (2 hunks)
  • strategy/trend/dema_strategy_test.go (2 hunks)
  • strategy/trend/envelope_strategy_test.go (2 hunks)
  • strategy/trend/golden_cross_strategy_test.go (2 hunks)
  • strategy/trend/kama_strategy_test.go (2 hunks)
  • strategy/trend/kdj_strategy_test.go (2 hunks)
  • strategy/trend/macd_strategy_test.go (2 hunks)
  • strategy/trend/qstick_strategy_test.go (2 hunks)
  • strategy/trend/smma_strategy_test.go (2 hunks)
  • strategy/trend/trima_strategy_test.go (2 hunks)
  • strategy/trend/triple_moving_average_crossover_strategy_test.go (2 hunks)
  • strategy/trend/trix_strategy_test.go (2 hunks)
  • strategy/trend/tsi_strategy_test.go (2 hunks)
  • strategy/trend/vwma_strategy_test.go (2 hunks)
  • strategy/trend/weighted_close_strategy_test.go (2 hunks)
  • strategy/volatility/bollinger_bands_strategy_test.go (2 hunks)
  • strategy/volatility/super_trend_strategy_test.go (2 hunks)
  • strategy/volume/chaikin_money_flow_strategy_test.go (2 hunks)
  • strategy/volume/ease_of_movement_strategy_test.go (2 hunks)
  • strategy/volume/force_index_strategy_test.go (2 hunks)
  • strategy/volume/money_flow_index_strategy_test.go (2 hunks)
  • strategy/volume/negative_volume_index_strategy_test.go (2 hunks)
  • strategy/volume/weighted_average_price_strategy_test.go (2 hunks)
  • trend/apo_test.go (1 hunks)
  • trend/aroon_test.go (1 hunks)
  • trend/bop_test.go (1 hunks)
  • trend/cci_test.go (1 hunks)
  • trend/envelope_test.go (2 hunks)
  • trend/hma_test.go (1 hunks)
  • trend/kama_test.go (1 hunks)
  • trend/kdj_test.go (1 hunks)
  • trend/macd_test.go (1 hunks)
  • trend/mass_index_test.go (1 hunks)
  • trend/mlr_test.go (1 hunks)
  • trend/mls_test.go (1 hunks)
  • trend/rma_test.go (1 hunks)
  • trend/smma_test.go (1 hunks)
  • trend/tema_test.go (1 hunks)
  • trend/trima_test.go (2 hunks)
  • trend/trix_test.go (1 hunks)
  • trend/tsi_test.go (1 hunks)
  • trend/typical_price_test.go (1 hunks)
  • trend/vwma_test.go (1 hunks)
  • trend/weighted_close_test.go (1 hunks)
  • trend/wma_test.go (1 hunks)
  • volatility/acceleration_bands_test.go (1 hunks)
⛔ Files not processed due to max files limit (22)
  • volatility/atr_test.go
  • volatility/bollinger_band_width_test.go
  • volatility/bollinger_bands_test.go
  • volatility/chandelier_exit_test.go
  • volatility/donchian_channel_test.go
  • volatility/keltner_channel_test.go
  • volatility/moving_std_test.go
  • volatility/percent_b_test.go
  • volatility/po_test.go
  • volatility/super_trend_test.go
  • volatility/ulcer_index_test.go
  • volume/ad_test.go
  • volume/cmf_test.go
  • volume/emv_test.go
  • volume/fi_test.go
  • volume/mfi_test.go
  • volume/mfm_test.go
  • volume/mfv_test.go
  • volume/nvi_test.go
  • volume/obv_test.go
  • volume/vpt_test.go
  • volume/vwap_test.go
🔇 Additional comments (80)
helper/README.md (2)

139-139: LGTM! Simplified default date format.

The simplified default date format aligns well with the PR objective of supporting configurable date time formats.


30-30: LGTM! Enhanced function signatures with options pattern.

The replacement of boolean parameters with variadic options improves flexibility and extensibility, aligning with the PR objective of enhancing CSV file configuration.

Also applies to: 72-72, 192-192, 815-815

trend/apo_test.go (1)

20-20: Verify CSV file structure and required options.

The removal of the hasHeader parameter suggests a shift to configurable CSV options. Please verify if the test data in testdata/apo.csv requires any specific CSV configuration (headers, date format, etc.).

✅ Verification successful

CSV file structure follows standard format, no special configuration needed

The test data in testdata/apo.csv follows a standard CSV format with headers and numeric values, which will be correctly handled by the default CSV configuration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check CSV file structure
head -n 5 trend/testdata/apo.csv

Length of output: 100

trend/trix_test.go (1)

20-20: Verify consistent CSV structure across test files.

The changes follow a consistent pattern of removing the hasHeader parameter. Let's verify that all test data files follow the same structure.

✅ Verification successful

CSV files structure is consistent

All test data files in the trend/testdata/ directory consistently include headers with proper column naming conventions, justifying the removal of the hasHeader parameter.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check structure consistency across test files
for f in trend/testdata/*.csv; do
  echo "=== $f ==="
  head -n 1 "$f"
done

Length of output: 3892

trend/tema_test.go (1)

20-20: Update documentation for CSV configuration changes.

The removal of the hasHeader parameter is part of a broader change to support configurable CSV options. Please ensure that the documentation is updated to reflect these changes.

✅ Verification successful

Documentation is already up-to-date with CSV configuration changes

The documentation in helper/README.md correctly reflects the new options-based API, including the WithoutCsvHeader option that replaces the previous hasHeader parameter. No further documentation updates are needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for documentation files
fd -e md -e txt . | xargs rg -l "ReadFromCsvFile|CSV|hasHeader"

Length of output: 124


Script:

#!/bin/bash
# Check content of documentation files for CSV-related information
echo "=== helper/README.md ==="
rg -A 3 -B 3 "ReadFromCsvFile|hasHeader|CSV" helper/README.md || true
echo -e "\n=== asset/README.md ==="
rg -A 3 -B 3 "ReadFromCsvFile|hasHeader|CSV" asset/README.md || true
echo -e "\n=== CONTRIBUTING.md ==="
rg -A 3 -B 3 "ReadFromCsvFile|hasHeader|CSV" CONTRIBUTING.md || true
echo -e "\n=== README.md ==="
rg -A 3 -B 3 "ReadFromCsvFile|hasHeader|CSV" README.md || true

Length of output: 5470

trend/rma_test.go (1)

20-20: Verify complete migration to new CSV configuration API.

Please ensure that all usages of ReadFromCsvFile have been updated to remove the hasHeader parameter.

✅ Verification successful

Migration to new CSV configuration API is complete

All usages of ReadFromCsvFile have been updated to remove the hasHeader parameter across the entire codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining uses of the old API
rg "ReadFromCsvFile.*,\s*true|ReadFromCsvFile.*,\s*false"

# Search for all test files that might need updating
fd "_test\.go$" | xargs rg "ReadFromCsvFile"

Length of output: 21458

momentum/rsi_test.go (1)

20-20: Verify CSV header handling for RSI test data.

The removal of the header parameter suggests a change in how CSV files are processed. Ensure that the test data in testdata/rsi.csv is still properly loaded with the new CSV configuration.

✅ Verification successful

CSV header handling for RSI test data is correct

The test data file has proper headers (Close,Rsi) and follows the expected CSV format, making it compatible with the updated CSV processing logic.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the structure of RSI test data file
# Expected: File should have headers for Close and Rsi columns

echo "Checking RSI test data structure:"
head -n 2 momentum/testdata/rsi.csv

Length of output: 137

trend/mlr_test.go (1)

21-21: Verify CSV structure matches MLR test data requirements.

The test expects specific columns (X, Y, R) in the CSV file. Ensure that the data structure in testdata/mlr.csv aligns with the new CSV configuration.

✅ Verification successful

CSV structure perfectly matches MLR test data requirements

The CSV file headers (X,Y,R) align exactly with the Data struct fields, and the numeric values are in the correct format.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the structure of MLR test data file
# Expected: File should have headers for X, Y, and R columns

echo "Checking MLR test data structure:"
head -n 2 trend/testdata/mlr.csv

Length of output: 124


Script:

#!/bin/bash
# Find and display the Data struct definition
ast-grep --pattern 'type Data struct {
  $$$
}'

Length of output: 10539

trend/vwma_test.go (1)

21-21: Verify type conversion for Volume field with new CSV configuration.

The test data includes an int64 Volume field. Ensure that the new CSV configuration correctly handles type conversion for non-float64 fields.

✅ Verification successful

Type conversion verification successful

The Volume field in vwma.csv contains valid integer values that are compatible with int64 type conversion. No further changes needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the structure and data types in VWMA test data
# Expected: File should have headers and proper numeric values for Close, Volume, and Vwma

echo "Checking VWMA test data structure and types:"
head -n 2 trend/testdata/vwma.csv

# Verify that Volume values are valid integers
echo "Checking Volume field values:"
awk -F',' 'NR>1 && $2 !~ /^[0-9]+$/ {print "Invalid Volume value at line " NR ": " $2}' trend/testdata/vwma.csv

Length of output: 354

momentum/awesome_oscillator_test.go (2)

21-21: Verify CSV structure and consider adding documentation.

The test relies on specific CSV data structure. Consider:

  1. Verify that the test data in testdata/awesome_oscillator.csv is compatible with the new CSV configuration
  2. Add documentation about the expected CSV format in the test file

Add documentation about the expected CSV format:

 func TestAwesomeOscillator(t *testing.T) {
+	// TestAwesomeOscillator verifies the Awesome Oscillator calculation
+	// using test data from testdata/awesome_oscillator.csv
+	// Expected CSV format:
+	// High,Low,Ao
+	// <float64>,<float64>,<float64>
 	type Data struct {
 		High float64
 		Low  float64
✅ Verification successful

CSV structure verified and compatible

The test data in awesome_oscillator.csv follows the correct structure with matching headers (High, Low, Ao) and appropriate float64 values. No changes needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the structure of Awesome Oscillator test data
# Expected: File should have headers for High, Low, and Ao columns

echo "Checking Awesome Oscillator test data structure:"
head -n 2 momentum/testdata/awesome_oscillator.csv

Length of output: 195


20-21: Consider applying new CSV configuration options to test files.

The PR introduces new CSV configuration options for headers and date-time formats, but these test files only show the removal of the hasHeader parameter. Consider if any of these tests would benefit from the new configuration options, particularly for consistent header handling across all test files.

Run the following to check for consistency in CSV headers across test files:

Also applies to: 21-21, 21-21, 21-21

momentum/qstick_test.go (1)

21-21: Verify CSV file structure and test behavior.

The removal of the hasHeader parameter suggests a change in how CSV headers are handled. Please ensure that:

  1. The test data in testdata/qstick.csv maintains its structure
  2. The test behavior remains unchanged with the new CSV configuration
✅ Verification successful

Change is safe - CSV header handling is correct

The removal of the hasHeader parameter is safe because:

  • The CSV file has proper headers ("Open,Close,Qstick")
  • The new implementation defaults to handling headers
  • The test behavior remains unchanged
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check CSV file structure
head -n 2 momentum/testdata/qstick.csv

# Verify if the file has headers
awk 'NR==1{print $0}' momentum/testdata/qstick.csv | grep -E '^[A-Za-z]'

Length of output: 175


Script:

#!/bin/bash
# Check the implementation of ReadFromCsvFile
ast-grep --pattern 'func ReadFromCsvFile[$_]($_) $_' helper/csv.go

Length of output: 68


Script:

#!/bin/bash
# Find files containing "ReadFromCsvFile"
rg -l "ReadFromCsvFile"

# Find all csv-related files as backup
fd -e go csv

Length of output: 3255


Script:

#!/bin/bash
# Check the implementation and its tests
cat helper/csv.go
cat helper/csv_test.go

Length of output: 15817

trend/mass_index_test.go (1)

21-21: Verify CSV file structure and test behavior.

The removal of the hasHeader parameter suggests a change in how CSV headers are handled. Please ensure that:

  1. The test data in testdata/mass_index.csv maintains its structure
  2. The test behavior remains unchanged with the new CSV configuration
✅ Verification successful

CSV file structure verified successfully

The CSV file maintains proper structure with appropriate headers (Open,Close,MassIndex) and numeric data, ensuring consistent test behavior with the new CSV handling implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check CSV file structure
head -n 2 trend/testdata/mass_index.csv

# Verify if the file has headers
awk 'NR==1{print $0}' trend/testdata/mass_index.csv | grep -E '^[A-Za-z]'

Length of output: 183

trend/wma_test.go (1)

20-20: Verify CSV file structure and test behavior.

The removal of the hasHeader parameter suggests a change in how CSV headers are handled. Please ensure that:

  1. The test data in testdata/wma.csv maintains its structure
  2. The test behavior remains unchanged with the new CSV configuration
✅ Verification successful

CSV file structure verified and maintains headers

The CSV file structure is correct with proper headers ("Close,Wma") and numeric data, ensuring test behavior remains consistent with the new CSV configuration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check CSV file structure
head -n 2 trend/testdata/wma.csv

# Verify if the file has headers
awk 'NR==1{print $0}' trend/testdata/wma.csv | grep -E '^[A-Za-z]'

Length of output: 136

trend/tsi_test.go (2)

20-20: Verify CSV file structure and test behavior.

The removal of the hasHeader parameter suggests a change in how CSV headers are handled. Please ensure that:

  1. The test data in testdata/tsi.csv maintains its structure
  2. The test behavior remains unchanged with the new CSV configuration
✅ Verification successful

CSV file structure verified successfully

The CSV file maintains its proper structure with headers (Close,Tsi) and correctly formatted data, supporting the removal of the hasHeader parameter.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check CSV file structure
head -n 2 trend/testdata/tsi.csv

# Verify if the file has headers
awk 'NR==1{print $0}' trend/testdata/tsi.csv | grep -E '^[A-Za-z]'

Length of output: 136


20-21: Document the new CSV configuration approach.

The consistent removal of the hasHeader parameter across all test files suggests a significant change in how CSV files are configured. To ensure maintainability:

  1. Please document how CSV headers are now handled by default
  2. Consider adding examples in test files demonstrating how to use the new CsvOption type for different configurations
  3. Verify that the default behavior matches the previous behavior with hasHeader=true
✅ Verification successful

CSV configuration is properly documented and implemented.

The new CSV configuration approach:

  • Maintains the same default behavior (headers enabled by default)
  • Uses a clean functional options pattern
  • Is well documented in helper/README.md with clear examples
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for documentation of new CSV configuration
rg -l "CsvOption|NewCsv" .

Length of output: 136


Script:

#!/bin/bash
# Check documentation in README files
echo "=== helper/README.md ==="
cat helper/README.md
echo -e "\n=== asset/README.md ==="
cat asset/README.md

# Check implementation and tests
echo -e "\n=== helper/csv.go ==="
cat helper/csv.go
echo -e "\n=== helper/csv_test.go ==="
cat helper/csv_test.go

Length of output: 71001


Script:

#!/bin/bash
# Check CSV implementation for default header behavior
rg -A 10 "type Csv\[T" helper/csv.go
rg -A 5 "NewCsv\[T" helper/csv.go

Length of output: 784

trend/smma_test.go (1)

20-20: Verify CSV format compatibility with new configuration.

The removal of the hasHeader parameter aligns with the PR's CSV configuration changes. However, since this test file reads from testdata/smma.csv, we should verify the CSV structure.

✅ Verification successful

CSV format is compatible with new configuration

The test data file has a proper header row and simple numeric columns, making it fully compatible with the refactored CSV handling.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the structure of the test data file to ensure compatibility
head -n 2 trend/testdata/smma.csv

Length of output: 59

trend/hma_test.go (1)

20-20: LGTM! Changes maintain consistency.

The removal of the hasHeader parameter is consistent with the new CSV configuration approach.

momentum/williams_r_test.go (1)

20-22: Add tests for new CSV configuration features.

While the changes consistently remove the hasHeader parameter, there are no visible tests for the new CSV configuration features mentioned in the PR objectives, particularly:

  • Different date time formats for snapshots
  • Custom headers configuration
  • Logging configuration

Consider adding dedicated test cases to verify these new features.

Would you like me to help create test cases for these new features?

trend/macd_test.go (1)

21-21: Verify CSV file structure matches default behavior.

The removal of the hasHeader parameter assumes the CSV file follows the default configuration. Please verify that testdata/macd.csv contains headers matching the Data struct fields.

✅ Verification successful

CSV file structure verified successfully

The headers in testdata/macd.csv perfectly match the Data struct fields, making the removal of the hasHeader parameter safe.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify CSV file structure
# Check if macd.csv exists and has expected headers
head -n 1 trend/testdata/macd.csv

Length of output: 53


Script:

#!/bin/bash
# Find and show Data struct definition
ast-grep --pattern 'type Data struct {
  $$$
}'

Length of output: 10539

trend/aroon_test.go (1)

24-24: Verify impact of CSV configuration changes.

The removal of the hasHeader parameter is part of a larger change to support configurable CSV options. Please ensure that:

  1. The default behavior matches the previous explicit true parameter
  2. The test remains valid with the new CSV configuration system
✅ Verification successful

CSV configuration changes are consistently applied

The removal of the hasHeader parameter is part of a broader refactoring that replaces the boolean parameter with a more flexible options pattern. This change has been consistently applied across all test files in the codebase that use ReadFromCsvFile.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify CSV handling changes
# Check for any other tests that might be affected by the CSV configuration changes
rg -l "ReadFromCsvFile.*true" .

Length of output: 33


Script:

#!/bin/bash
# Find all ReadFromCsvFile usages
rg "ReadFromCsvFile\[.*\]" -A 2

# Check helper package implementation
fd "helper.*go" --type f --exec cat {}

Length of output: 42379

strategy/trend/kdj_strategy_test.go (1)

17-17: Changes consistent with CSV configuration updates.

The modifications align with the broader refactoring of CSV handling.

Also applies to: 22-22

strategy/trend/cci_strategy_test.go (1)

17-17: Changes consistent with CSV configuration updates.

The modifications align with the broader refactoring of CSV handling.

Also applies to: 22-22

strategy/trend/tsi_strategy_test.go (1)

17-17: Changes consistent with CSV configuration updates.

The modifications align with the broader refactoring of CSV handling.

Let's verify if all strategy test files have been updated consistently:

Also applies to: 22-22

✅ Verification successful

CSV refactoring verification successful

All strategy test files consistently use the new generic CSV reading pattern without boolean parameters.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find any remaining instances of ReadFromCsvFile with boolean parameter
echo "=== Checking for remaining boolean parameters in ReadFromCsvFile calls ==="
rg -l "ReadFromCsvFile\[[^\]]+\]\([^,]+,\s*true\)" strategy/

Length of output: 221


Script:

#!/bin/bash
# Find all test files in strategy directory
echo "=== Strategy Test Files ==="
fd "_test\.go$" strategy/

# Check all CSV file reading patterns
echo -e "\n=== CSV Reading Patterns ==="
rg "ReadFromCsvFile\[.*?\]" strategy/ -A 1

Length of output: 24708

strategy/trend/dema_strategy_test.go (1)

17-17: Verify CSV file headers after removing hasHeader parameter.

The removal of the hasHeader boolean parameter from ReadFromCsvFile calls suggests a change in how CSV headers are handled. Please verify that the test data files ("testdata/brk-b.csv" and "testdata/dema_strategy.csv") are compatible with the new default behavior.

Also applies to: 22-22, 39-39

✅ Verification successful

CSV files are compatible with the new header handling

Both test data files have proper CSV headers:

  • strategy/trend/testdata/brk-b.csv includes standard OHLCV headers
  • strategy/trend/testdata/dema_strategy.csv includes the "Action" header
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if test data files have headers and verify their structure

echo "Checking brk-b.csv headers:"
head -n 1 testdata/brk-b.csv

echo -e "\nChecking dema_strategy.csv headers:"
head -n 1 testdata/dema_strategy.csv

Length of output: 385


Script:

#!/bin/bash
# Find CSV files in the repository
echo "Searching for brk-b.csv:"
fd "brk-b.csv"

echo -e "\nSearching for dema_strategy.csv:"
fd "dema_strategy.csv"

# If files are found, we'll check their headers in the next iteration

Length of output: 553


Script:

#!/bin/bash
echo "Content of strategy/trend/testdata/brk-b.csv (first 2 lines):"
head -n 2 strategy/trend/testdata/brk-b.csv

echo -e "\nContent of strategy/trend/testdata/dema_strategy.csv (first 2 lines):"
head -n 2 strategy/trend/testdata/dema_strategy.csv

Length of output: 514

strategy/trend/smma_strategy_test.go (1)

22-22: Verify smma_strategy.csv headers after removing hasHeader parameter.

Please verify that "testdata/smma_strategy.csv" is compatible with the new default header behavior.

✅ Verification successful

CSV file is compatible with new header behavior

The file "strategy/trend/testdata/smma_strategy.csv" has a proper header row ("Action") and is compatible with the new default header behavior. No changes are needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check smma_strategy.csv headers

echo "Checking smma_strategy.csv headers:"
head -n 1 testdata/smma_strategy.csv

Length of output: 205


Script:

#!/bin/bash
# Search for smma strategy CSV files
echo "Searching for SMMA strategy CSV files:"
fd -t f "smma.*\.csv"

# Also search in case the file exists but with a different naming pattern
echo -e "\nSearching for all CSV files:"
fd -t f -e csv

Length of output: 3982


Script:

#!/bin/bash
# Check the first few lines of the SMMA strategy CSV file
echo "SMMA strategy CSV file contents (first 3 lines):"
head -n 3 strategy/trend/testdata/smma_strategy.csv

Length of output: 171

strategy/trend/trix_strategy_test.go (1)

17-17: Verify documentation for the new CSV configuration.

Since the hasHeader parameter has been removed in favor of the new CsvOption type, please ensure:

  1. The changes are documented in the package documentation
  2. A migration guide exists for users to update their code

Also applies to: 22-22, 39-39

✅ Verification successful

Documentation and migration path are properly implemented.

The CsvOption type and its options are well-documented in the package documentation:

  • WithoutCsvHeader() provides a direct replacement for hasHeader=false
  • Default behavior (with header) remains unchanged
  • Additional options (WithCsvLogger, WithCsvDefaultDateTimeFormat) enhance functionality
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for documentation updates

# Check for package documentation
echo "Checking package documentation:"
rg -A 5 "CsvOption" "**/*.go"

# Check for migration guide or breaking changes documentation
echo -e "\nChecking for migration guide:"
fd -g "*.md" | xargs rg -l "CSV|breaking changes"

Length of output: 336


Script:

#!/bin/bash
# Check the content of relevant README files
echo "=== Content of helper/README.md ==="
cat helper/README.md

echo -e "\n=== Looking for CsvOption implementation ==="
# Search for CsvOption type definition and related documentation
rg -A 10 "type CsvOption" .

echo -e "\n=== Searching for CSV-related function signatures ==="
ast-grep --pattern 'func ReadFromCsvFile[$_]($_) $_'

Length of output: 46722

strategy/trend/macd_strategy_test.go (1)

17-17: Verify completeness of CSV refactoring across all test files.

Please ensure that all test files using ReadFromCsvFile have been updated to remove the hasHeader parameter.

Also applies to: 22-22, 39-39

✅ Verification successful

CSV refactoring is complete across all test files

The verification shows that all 91 test files using ReadFromCsvFile have been updated to remove the hasHeader parameter. No instances of the old signature remain in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining uses of the old ReadFromCsvFile signature

# Search for test files still using the old signature
echo "Checking for remaining hasHeader usage in tests:"
rg "ReadFromCsvFile.*,\s*true" "**/*_test.go"

# List all test files using ReadFromCsvFile to ensure none were missed
echo -e "\nAll test files using ReadFromCsvFile:"
rg -l "ReadFromCsvFile" "**/*_test.go"

Length of output: 390


Script:

#!/bin/bash
# Description: Check for any remaining uses of the old ReadFromCsvFile signature with proper path handling

# Search for test files still using the old signature
echo "Checking for remaining hasHeader usage in tests:"
rg "ReadFromCsvFile.*,\s*true" --type-add 'gotest:*_test.go' -t gotest

# List all test files using ReadFromCsvFile to ensure none were missed
echo -e "\nAll test files using ReadFromCsvFile:"
rg -l "ReadFromCsvFile" --type-add 'gotest:*_test.go' -t gotest

Length of output: 3455

strategy/momentum/rsi_strategy_test.go (1)

17-17: Verify CSV file headers in test data.

The removal of the hasHeader parameter suggests a change in how CSV headers are handled. Please ensure that the test data files ("testdata/brk-b.csv", "testdata/rsi_strategy.csv") maintain their structure and are compatible with the new CSV configuration mechanism.

Also applies to: 22-22, 39-39

✅ Verification successful

CSV files consistently use headers across the codebase

The removal of the hasHeader parameter is safe as all test data files, including "brk-b.csv" and "rsi_strategy.csv", consistently maintain their header structure. This change aligns with the standardized CSV handling across the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the structure of test data files to ensure they have headers
# and the correct date format.

echo "Checking brk-b.csv structure:"
head -n 1 testdata/brk-b.csv

echo "Checking rsi_strategy.csv structure:"
head -n 1 testdata/rsi_strategy.csv

Length of output: 383


Script:

#!/bin/bash
# Find all CSV files in the repository
echo "Searching for CSV files:"
fd -e csv

# If files are found, check their content
for file in $(fd -e csv); do
  echo -e "\nContents of $file:"
  head -n 2 "$file"
done

Length of output: 29390

strategy/trend/trima_strategy_test.go (1)

17-17: Changes align with the CSV configuration refactoring.

The removal of the hasHeader parameter is consistent with the changes in other test files.

Also applies to: 22-22, 39-39

strategy/trend/aroon_strategy_test.go (1)

17-17: Changes align with the CSV configuration refactoring.

The removal of the hasHeader parameter is consistent with the changes in other test files.

Also applies to: 22-22, 39-39

strategy/buy_and_hold_strategy_test.go (1)

16-16: Consider consolidating test data locations.

While the changes align with the CSV configuration refactoring, I notice that this test uses data from "testdata/repository/brk-b.csv" while other tests use "testdata/brk-b.csv". Consider consolidating test data to a single location to improve maintainability.

Also, verify the CSV structure:

Also applies to: 21-21, 38-38

strategy/trend/qstick_strategy_test.go (1)

17-17: Verify CSV file headers after configuration changes.

The removal of the hasHeader parameter suggests a move to a configurable approach. Please verify that the test CSV files (brk-b.csv, qstick_strategy.csv) have headers, and if needed, configure the CSV options explicitly.

Also applies to: 22-22, 39-39

✅ Verification successful

CSV files have appropriate headers, no configuration needed

Both test files (brk-b.csv and qstick_strategy.csv) have proper headers, confirming that the removal of the hasHeader parameter is safe. No additional configuration is required.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if test CSV files have headers
# Expected: First line should contain column names

echo "Checking brk-b.csv headers:"
head -n 1 strategy/trend/testdata/brk-b.csv

echo -e "\nChecking qstick_strategy.csv headers:"
head -n 1 strategy/trend/testdata/qstick_strategy.csv

Length of output: 307

strategy/compound/macd_rsi_strategy_test.go (1)

17-17: Verify CSV file headers after configuration changes.

The removal of the hasHeader parameter suggests a move to a configurable approach. Please verify that the test CSV files (brk-b.csv, macd_rsi_strategy.csv) have headers, and if needed, configure the CSV options explicitly.

Also applies to: 22-22, 39-39

✅ Verification successful

CSV files confirmed to have proper headers

Both test files have valid headers:

  • brk-b.csv: Contains standard OHLCV columns
  • macd_rsi_strategy.csv: Contains Action column
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if test CSV files have headers
# Expected: First line should contain column names

echo "Checking brk-b.csv headers:"
head -n 1 strategy/compound/testdata/brk-b.csv

echo -e "\nChecking macd_rsi_strategy.csv headers:"
head -n 1 strategy/compound/testdata/macd_rsi_strategy.csv

Length of output: 319

strategy/momentum/triple_rsi_strategy_test.go (1)

17-17: Verify CSV header handling after parameter removal.

The removal of the true parameter suggests these CSV files have headers. According to the PR objectives, helper.NewCsv now supports configurable options. Consider explicitly setting the header option to maintain the original behavior:

csvOpts := helper.NewCsvOptions().WithHeaders(true)
snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv", csvOpts)

Let's verify the CSV file structure:

Also applies to: 22-22, 39-39

✅ Verification successful

The removal of the header parameter is safe

The CSV files are confirmed to have headers, and the new ReadFromCsvFile implementation handles headers by default. No changes are needed as the behavior is preserved without the explicit parameter.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the CSV files have headers
head -n 1 strategy/momentum/testdata/brk-b.csv strategy/momentum/testdata/triple_rsi_strategy.csv

Length of output: 254


Script:

#!/bin/bash
# Find ReadFromCsvFile implementation
ast-grep --pattern 'func ReadFromCsvFile[$_]($_) $_'

Length of output: 54


Script:

#!/bin/bash
# Find ReadFromCsvFile implementation using ripgrep
rg -A 10 "func ReadFromCsvFile"

Length of output: 2195


Script:

#!/bin/bash
# Find NewCsv and CsvOption implementation
rg -A 15 "func NewCsv|type CsvOption"

Length of output: 5123

strategy/volume/ease_of_movement_strategy_test.go (1)

17-17: Same header handling concerns as triple_rsi_strategy_test.go.

Also applies to: 22-22, 39-39

strategy/volume/money_flow_index_strategy_test.go (1)

17-17: Same header handling concerns as triple_rsi_strategy_test.go.

Also applies to: 22-22, 39-39

strategy/volatility/bollinger_bands_strategy_test.go (1)

17-17: Same header handling concerns as triple_rsi_strategy_test.go.

Also applies to: 22-22, 39-39

strategy/volume/chaikin_money_flow_strategy_test.go (1)

17-17: Verify CSV file compatibility with the new configuration.

The removal of the hasHeader parameter suggests these CSV files are now processed using default options. Please ensure:

  1. The test data files (brk-b.csv, chaikin_money_flow_strategy.csv) are compatible with the new CSV handling
  2. The header structure matches the expected format for both asset.Snapshot and strategy.Result types

Also applies to: 22-22, 39-39

✅ Verification successful

CSV files are compatible with default configuration

Both test data files have appropriate headers that match their respective types:

  • brk-b.csv: Contains standard financial data headers (Date, Open, High, Low, Close, Adj Close, Volume)
  • chaikin_money_flow_strategy.csv: Contains the Action header for strategy results
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify CSV file structure matches expected types

# Check if test files exist and have headers
echo "Checking brk-b.csv headers:"
head -n 1 strategy/volume/testdata/brk-b.csv

echo "Checking chaikin_money_flow_strategy.csv headers:"
head -n 1 strategy/volume/testdata/chaikin_money_flow_strategy.csv

Length of output: 339

strategy/volatility/super_trend_strategy_test.go (1)

17-17: Review previously applies.

The changes are identical to those in chaikin_money_flow_strategy_test.go. The same verification steps apply.

Also applies to: 22-22, 39-39

strategy/momentum/awesome_oscillator_strategy_test.go (1)

17-17: Review previously applies.

The changes are identical to those in previous strategy test files. The same verification steps apply.

Also applies to: 22-22, 39-39

momentum/stochastic_oscillator_test.go (1)

23-23: Verify custom struct mapping with new CSV configuration.

This test uses a custom Data struct with specific fields (High, Low, Close, K, D). Please ensure:

  1. The CSV columns correctly map to struct fields with the new configuration
  2. The field types match the expected CSV data format
✅ Verification successful

Custom struct mapping verified successfully

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify CSV structure matches custom Data struct

# Check CSV headers and first data row
echo "Checking stochastic_oscillator.csv structure:"
head -n 2 momentum/testdata/stochastic_oscillator.csv

Length of output: 212


Script:

#!/bin/bash
# Find and show Data struct definition
ast-grep --pattern 'type Data struct {
  $$$
}'

Length of output: 10539

strategy/trend/weighted_close_strategy_test.go (1)

17-19: Verify CSV file structure and consider adding configuration options.

Given the changes to make CSV handling more configurable, consider:

  1. Verifying that the test data files (brk-b.csv and weighted_close_strategy.csv) still work correctly without the header configuration
  2. Adding date format configuration if the test data uses non-default date formats

Also applies to: 22-24

✅ Verification successful

CSV files are properly structured and compatible with default settings

The test data files are well-formatted with proper headers and use standard ISO 8601 date format (YYYY-MM-DD). No additional configuration is needed for header handling or date parsing.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the structure of test data files
echo "=== brk-b.csv header ==="
head -n 1 strategy/trend/testdata/brk-b.csv
echo "=== weighted_close_strategy.csv header ==="
head -n 1 strategy/trend/testdata/weighted_close_strategy.csv

Length of output: 313


Script:

#!/bin/bash
# Check the first few data rows
echo "=== brk-b.csv first 3 rows ==="
head -n 4 strategy/trend/testdata/brk-b.csv
echo -e "\n=== weighted_close_strategy.csv first 3 rows ==="
head -n 4 strategy/trend/testdata/weighted_close_strategy.csv

Length of output: 562

strategy/momentum/stochastic_rsi_strategy_test.go (1)

17-19: Changes are consistent with the refactoring pattern.

The modifications align with the removal of the boolean parameter seen in other files.

Also applies to: 22-24

strategy/volume/negative_volume_index_strategy_test.go (1)

17-19: Changes are consistent with the refactoring pattern.

The modifications align with the removal of the boolean parameter seen in other files.

Also applies to: 22-24

strategy/volume/weighted_average_price_strategy_test.go (1)

17-19: Changes are consistent with the refactoring pattern.

The modifications align with the removal of the boolean parameter seen in other files.

Also applies to: 22-24

trend/weighted_close_test.go (1)

22-22: Verify CSV configuration requirements for test data.

The removal of the boolean parameter from ReadFromCsvFile aligns with the new CSV configuration approach. However, we should verify if this test data requires any specific CSV configuration (e.g., custom date format, headers).

✅ Verification successful

CSV structure is standard, no special configuration needed

The test data in weighted_close.csv has a simple structure with standard numeric values and clear headers, requiring no special CSV configuration. The removal of the boolean parameter is safe in this context.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check CSV file structure and potential configuration needs
head -n 5 trend/testdata/weighted_close.csv

Length of output: 229

strategy/trend/triple_moving_average_crossover_strategy_test.go (2)

17-17: Verify date format handling in brk-b.csv.

Since the asset.Snapshot.Date field no longer has a declared date format, ensure that the test data in brk-b.csv is compatible with the default date format or configure an appropriate format using the new CSV options.

✅ Verification successful

Date format in brk-b.csv is compatible with default parsers

The CSV file uses the standard ISO 8601 date format (YYYY-MM-DD), which is compatible with default date parsers and requires no special configuration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check date format in the CSV file
head -n 5 strategy/trend/testdata/brk-b.csv

Length of output: 375


22-22: Verify date format in strategy results CSV.

Ensure that the date format in the strategy results CSV matches the format used in the snapshots for consistent comparison.

momentum/chaikin_oscillator_test.go (1)

24-24: LGTM! Simple numeric data doesn't require special CSV configuration.

The removal of the boolean parameter is safe here as the test data consists of simple numeric fields without any date formatting requirements.

momentum/ppo_test.go (1)

22-22: Verify CSV header handling after parameter removal.

The removal of the header parameter from ReadFromCsvFile might affect how the test data is parsed. Please ensure that the CSV file structure in testdata/ppo.csv matches the expected behavior of the new CSV configuration system.

✅ Verification successful

CSV header handling is properly managed by the helper package

The removal of the header parameter is safe as the CSV helper package now uses a more flexible configuration system with functional options. By default, CSV files are read with headers, and tests can explicitly opt out using WithoutCsvHeader when needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check the structure of the test CSV file
head -n 2 testdata/ppo.csv

# Verify if other test files in the same directory follow the same structure
echo "CSV files in testdata directory:"
find testdata -name "*.csv" -exec head -n 1 {} \;

Length of output: 278


Script:

#!/bin/bash
# Search for CSV files in the repository
echo "Looking for CSV files:"
fd -e csv

# Search for references to CSV files in Go files
echo -e "\nReferences to CSV files in Go code:"
rg -t go ".*\.csv"

# Search for test data directory references
echo -e "\nReferences to testdata directory:"
rg "testdata"

Length of output: 48721


Script:

#!/bin/bash
# Check CSV helper implementation
echo "CSV helper implementation:"
rg -A 10 "func ReadFromCsvFile" helper/

# Check for CSV configuration or options
echo -e "\nCSV configuration:"
rg -A 5 "type.*CsvOption" helper/
rg -A 5 "WithHeader" helper/

Length of output: 3684

momentum/pvo_test.go (1)

22-22: Same header verification needed as in ppo_test.go.

This change is consistent with other test files. Please ensure the CSV structure verification covers this file as well.

strategy/decorator/no_loss_strategy_test.go (1)

23-23: Verify CSV structure compatibility.

Ensure that the strategy results CSV file structure is compatible with the new CSV configuration system.

✅ Verification successful

CSV structure is compatible with the configuration system

The no_loss_strategy.csv file follows a valid format that's fully compatible with the helper.ReadFromCsvFile generic implementation, which supports both header and headerless CSV files through its configurable options system.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check both CSV files used in this test
echo "Structure of brk-b.csv:"
head -n 2 testdata/brk-b.csv
echo -e "\nStructure of no_loss_strategy.csv:"
head -n 2 testdata/no_loss_strategy.csv

Length of output: 381


Script:

#!/bin/bash
# Find all CSV files in the repository
echo "Looking for CSV files:"
fd -e csv

# Check the helper package implementation
echo -e "\nHelper package implementation:"
ast-grep --pattern 'func ReadFromCsvFile[$_]($_) ($_,$_) {
  $$$
}'

Length of output: 4227


Script:

#!/bin/bash
# Check helper test files structure
echo "Helper test CSV files structure:"
head -n 2 helper/testdata/with_header.csv
head -n 2 helper/testdata/without_header.csv

# Check strategy CSV file structure
echo -e "\nStrategy CSV file structure:"
head -n 2 strategy/decorator/testdata/no_loss_strategy.csv

# Look for helper implementation
echo -e "\nHelper implementation:"
rg -A 5 "func ReadFromCsvFile" 

Length of output: 1702

strategy/decorator/stop_loss_strategy_test.go (1)

18-18: Verify CSV file structure after header handling changes.

The removal of the hasHeader parameter suggests a change in how CSV headers are handled. Please verify that the test data files "brk-b.csv" and "stop_loss_strategy.csv" maintain their expected structure with headers.

Also applies to: 23-23

✅ Verification successful

CSV files maintain correct header structure

Both test data files have proper headers and maintain their expected structure:

  • brk-b.csv includes standard OHLCV headers
  • stop_loss_strategy.csv includes the "Action" header
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify CSV file structure for test data
# Expected: First line should contain headers

echo "Checking brk-b.csv headers:"
head -n 1 testdata/brk-b.csv

echo -e "\nChecking stop_loss_strategy.csv headers:"
head -n 1 testdata/stop_loss_strategy.csv

Length of output: 405


Script:

#!/bin/bash
# Find all CSV files in the repository
echo "Searching for CSV files:"
fd -e csv

echo -e "\nChecking directory structure around the test file:"
fd strategy/decorator

Length of output: 4603


Script:

#!/bin/bash
echo "Content of strategy/decorator/testdata/brk-b.csv (first 2 lines):"
head -n 2 strategy/decorator/testdata/brk-b.csv

echo -e "\nContent of strategy/decorator/testdata/stop_loss_strategy.csv (first 2 lines):"
head -n 2 strategy/decorator/testdata/stop_loss_strategy.csv

Length of output: 553

asset/snapshot_test.go (1)

15-15: Verify date parsing behavior with new CSV configuration.

Since the PR introduces configurable date formats and removes the date format struct tag from asset.Snapshot.Date, ensure that this test still correctly parses dates from the CSV file. Consider adding explicit tests for different date formats.

✅ Verification successful

Test will continue to work with standard ISO date format

The CSV file uses the standard ISO 8601 date format (YYYY-MM-DD) which is natively supported by Go's time.Parse, making the format struct tag unnecessary. The test data will continue to parse correctly without any changes needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check date format in test data
# Expected: Consistent date format across entries

echo "Checking date format in brk-b.csv:"
# Extract date column (assuming it's the first column) and show unique formats
awk -F',' 'NR>1 {print $1}' testdata/repository/brk-b.csv | sort -u | head -n 5

Length of output: 258


Script:

#!/bin/bash
# First, find the CSV file location
echo "Searching for brk-b.csv:"
fd -t f "brk-b.csv"

# Find and show the test file to understand the context
echo -e "\nLocating snapshot_test.go files:"
fd -t f "snapshot_test.go"

# If found, show the relevant test context
echo -e "\nShowing test context:"
rg -C 5 "ReadFromCsvFile.*brk-b.csv" --type go

Length of output: 61464


Script:

#!/bin/bash
# Find and show Snapshot struct definition
echo "Snapshot struct definition:"
ast-grep --pattern 'type Snapshot struct {
  $$$
}' --lang go

# Show first few lines of CSV file to check date format
echo -e "\nCSV file contents:"
head -n 5 asset/testdata/repository/brk-b.csv

Length of output: 1609

strategy/majority_strategy_test.go (1)

17-17: Similar changes to stop_loss_strategy_test.go

The modifications follow the same pattern of removing the hasHeader parameter. The concerns and verifications from the stop loss strategy test apply here as well.

Also applies to: 22-22

volatility/acceleration_bands_test.go (1)

24-24: Verify numeric data parsing with new CSV configuration.

The test relies on accurate parsing of floating-point values. Ensure that the new CSV configuration maintains precise numeric parsing, especially for the acceleration bands calculations that require high accuracy.

✅ Verification successful

Numeric data parsing verified and confirmed accurate.

The CSV file maintains 6-decimal precision for all numeric values, which is properly handled by the float64 fields in the Data struct. The header row ensures correct column mapping to struct fields.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify numeric precision in test data
# Expected: Consistent decimal precision

echo "Checking numeric precision in acceleration_bands.csv:"
# Show a sample of numeric values to verify precision
head -n 5 testdata/acceleration_bands.csv

Length of output: 251


Script:

#!/bin/bash
# Find the acceleration bands CSV file
fd -e csv acceleration_bands

# Check the test file content for CSV path references
rg -A 2 -B 2 "acceleration_bands.csv" .

Length of output: 428


Script:

#!/bin/bash
# Check CSV file content and format
echo "Examining acceleration bands test data:"
head -n 5 volatility/testdata/acceleration_bands.csv

# Show the Data type structure to understand expected format
echo -e "\nChecking Data type definition:"
rg -A 5 "type Data struct" volatility/acceleration_bands_test.go

Length of output: 578

strategy/split_strategy_test.go (1)

17-17: Verify CSV file date format handling.

With the removal of the boolean parameter and the introduction of configurable date formats, we should ensure that:

  1. The default date format correctly handles the dates in "brk-b.csv"
  2. The test covers scenarios with different date formats

Let's verify the CSV file structure and date format:

Also applies to: 22-22

✅ Verification successful

CSV date format handling is correctly implemented

The CSV files use standard ISO 8601 date format (YYYY-MM-DD) which is well-supported by default. The removal of the boolean parameter is safe as it doesn't affect the parsing of these test files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the date format in the test CSV files
echo "Checking brk-b.csv date format:"
head -n 2 testdata/repository/brk-b.csv

echo -e "\nChecking split.csv date format:"
head -n 2 testdata/split.csv

Length of output: 391


Script:

#!/bin/bash
# Find CSV files by name
echo "Searching for brk-b.csv:"
fd "brk-b.csv"

echo -e "\nSearching for split.csv:"
fd "split.csv"

Length of output: 515


Script:

#!/bin/bash
# Check contents of the relevant CSV files
echo "Checking strategy/testdata/repository/brk-b.csv format:"
head -n 2 strategy/testdata/repository/brk-b.csv

echo -e "\nChecking strategy/testdata/split.csv format:"
head -n 2 strategy/testdata/split.csv

Length of output: 443

strategy/and_strategy_test.go (1)

16-16: LGTM! Changes align with the new CSV configuration approach.

The removal of the boolean parameter is consistent with the changes in other test files.

The date format verification for "brk-b.csv" was already covered in the review of split_strategy_test.go.

Also applies to: 21-21

helper/report_test.go (1)

Line range hint 14-20: Verify date format tag behavior with new CSV configuration.

The Row struct uses a date format tag (format:"2006-01-02"). We should verify that this tag is still respected with the new CSV configuration approach.

Let's check the CSV file structure and verify the date parsing:

Also applies to: 23-23

✅ Verification successful

Date format tag is properly handled

The format tag format:"2006-01-02" is fully supported by the CSV implementation:

  • The CSV parser explicitly handles format tags during initialization
  • The default date format matches the tag (2006-01-02)
  • Test data format (2022-11-30) confirms proper parsing
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the date format in report.csv matches the struct tag
echo "Checking report.csv date format:"
head -n 2 testdata/report.csv

Length of output: 185


Script:

#!/bin/bash
# Find CSV files in the repository
echo "Looking for CSV files:"
fd -e csv

# Search for code handling date format tags
echo -e "\nChecking date format handling code:"
rg -A 5 'format:.*2006-01-02' --type go

Length of output: 4738


Script:

#!/bin/bash
# Check the content of the test CSV file
echo "Content of helper/testdata/report.csv:"
head -n 3 helper/testdata/report.csv

# Look for CSV parsing code
echo -e "\nChecking CSV parsing implementation:"
ast-grep --pattern 'func $_($$$ *csv.$$$) $$$'
rg -l "encoding/csv" --type go

Length of output: 511


Script:

#!/bin/bash
# Check CSV parsing implementation
echo "CSV parsing implementation:"
cat helper/csv.go

# Look for struct tag handling
echo -e "\nStruct tag handling:"
rg "format.*tag" --type go -A 5

Length of output: 8586

momentum/ichimoku_cloud_test.go (1)

26-26: LGTM! Simple removal of the boolean parameter.

The change is straightforward and doesn't affect date handling since the Data struct doesn't contain date fields.

asset/snapshot.go (1)

17-17: Verify backward compatibility after removing the date format tag.

The removal of the format:"2006-01-02" tag allows for flexible date formats, which aligns with the PR objectives. However, we should ensure existing CSV files with dates in the old format continue to work.

✅ Verification successful

Removal of date format tag is safe and intentional

The change aligns with the PR's objective to support configurable date formats. The existing test files with dates in YYYY-MM-DD format continue to work through the generic CSV parsing helper, indicating backward compatibility is maintained.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for existing CSV files with dates in the old format
rg -l '\d{4}-\d{2}-\d{2}' --type csv

Length of output: 193


Script:

#!/bin/bash
# Check test files for date parsing related tests
rg -B2 -A2 "2006-01-02|time.Parse" --type go asset/

Length of output: 477


Script:

#!/bin/bash
# Check CSV parsing implementation
ast-grep --pattern 'func $_($_, $_) (*Snapshot, error)' asset/

Length of output: 64


Script:

#!/bin/bash
# Look for CSV parsing related code
rg -B2 -A2 "csv\." --type go asset/

Length of output: 37


Script:

#!/bin/bash
# Look for Snapshot-related test code
rg -B2 -A2 "func Test.*Snapshot" asset/

Length of output: 304

trend/envelope_test.go (1)

22-22: LGTM! API migration completed correctly.

The test has been properly updated to use the new CSV file reading API.

Also applies to: 58-58

helper/csv.go (5)

50-52: LGTM! Well-structured addition of date format customization.

The new defaultDateTimeFormat field enables per-instance customization while maintaining encapsulation.


Line range hint 79-121: LGTM! Clean implementation of the options pattern.

The function properly handles defaults and safely applies options. The thread-safety is maintained as each instance gets its own configuration.


Line range hint 323-340: LGTM! Good parameter ordering.

The function follows Go conventions by placing the variadic options parameter last.


27-27: Verify the impact of date format change on existing code.

The default date format has been simplified to exclude time components. While this aligns with the PR objectives, it could affect existing code that expects time components.

✅ Verification successful

Change to date-only format is safe ✅

The format change is safe as it's only used as an initial value and can be overridden using WithCsvDefaultDateTimeFormat option. Test cases already use custom format when time components are needed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usages of DefaultDateTimeFormat to identify potential impacts
rg -l "DefaultDateTimeFormat" | while read -r file; do
  echo "=== $file ==="
  rg -A 5 "DefaultDateTimeFormat" "$file"
done

Length of output: 2124


Line range hint 310-317: Add migration guide for the breaking change.

The removal of the hasHeader parameter is a breaking change. Please add a migration guide in the README or documentation to help users transition from the boolean parameter to the new options pattern.

momentum/stochastic_rsi_test.go (1)

20-20: LGTM! Consistent update to use the new API.

The test has been correctly updated to use the new ReadFromCsvFile signature.

trend/cci_test.go (1)

22-22: LGTM! Consistent update to use the new API.

The test has been correctly updated to use the new ReadFromCsvFile signature.

strategy/trend/apo_strategy_test.go (1)

17-17: LGTM! Consistent update to use the new API.

All calls to ReadFromCsvFile have been correctly updated to use the new signature.

Also applies to: 22-22, 39-39

strategy/trend/kama_strategy_test.go (1)

17-17: Verify CSV file headers in test data.

The removal of the hasHeader parameter suggests that header handling is now configured differently. Please verify that the test data files "brk-b.csv" and "kama_strategy.csv" are structured correctly with headers, as they were previously read with hasHeader=true.

Also applies to: 22-22

✅ Verification successful

CSV files contain expected headers

Both test data files have appropriate headers:

  • brk-b.csv: Contains standard OHLCV data columns
  • kama_strategy.csv: Contains Action column
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if test data files have headers
echo "First line of brk-b.csv:"
head -n 1 strategy/trend/testdata/brk-b.csv

echo -e "\nFirst line of kama_strategy.csv:"
head -n 1 strategy/trend/testdata/kama_strategy.csv

Length of output: 289

strategy/trend/vwma_strategy_test.go (1)

17-17: Same changes as in kama_strategy_test.go

Also applies to: 39-39

strategy/volume/force_index_strategy_test.go (1)

17-17: Same changes as in previous strategy test files

Also applies to: 39-39

strategy/trend/envelope_strategy_test.go (1)

17-17: Same changes as in previous strategy test files

Also applies to: 39-39

trend/kama_test.go (1)

Line range hint 14-20: Verify struct field mapping with new CSV configuration.

The test uses a custom Data struct with specific fields (Close and Kama). Please ensure that the new CSV configuration mechanism correctly maps these struct fields without the hasHeader parameter.

✅ Verification successful

CSV structure verified - field mapping is compatible

The CSV file has headers that exactly match the struct field names (Close and Kama), ensuring correct field mapping with the new CSV configuration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check CSV structure and field mapping
echo "Checking kama.csv structure:"
head -n 2 trend/testdata/kama.csv

Length of output: 122

strategy/decorator/inverse_strategy_test.go (1)

18-18: LGTM! Consistent with the new CSV configuration approach.

The removal of the boolean parameter from ReadFromCsvFile calls aligns with the PR's objective of making CSV configuration more flexible through options.

Let's verify that all test files have been updated consistently:

Also applies to: 23-23

✅ Verification successful

Verification complete: CSV reader changes are consistent

The removal of the boolean parameter from ReadFromCsvFile has been properly implemented and all usages across the codebase have been updated to use the new options-based pattern.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining instances of ReadFromCsvFile with boolean parameter
# Expect: No matches as all calls should have been updated

rg -p 'ReadFromCsvFile\[[^\]]+\]\([^,\)]+,\s*true\)' || echo "All files updated correctly"

Length of output: 119


Script:

#!/bin/bash
# Check for any remaining instances with 'false' parameter
rg -p 'ReadFromCsvFile\[[^\]]+\]\([^,\)]+,\s*false\)'

# Find all test files using ReadFromCsvFile
rg -p 'ReadFromCsvFile\[[^\]]+\]' --type go --type-add 'go:*.go' -l

# Check the implementation of ReadFromCsvFile
ast-grep --pattern 'func ReadFromCsvFile[$$$]'

Length of output: 3341


Script:

#!/bin/bash
# Check the implementation and surrounding context
rg -p 'func ReadFromCsvFile' helper/csv.go -B 2 -A 10

Length of output: 665

trend/trima_test.go (1)

20-20: LGTM! Consistent with the new CSV configuration approach.

The removal of the boolean parameter from ReadFromCsvFile calls maintains consistency with the new options-based configuration pattern.

Also applies to: 49-49

asset/README.md (2)

37-37: LGTM! Documentation accurately reflects the new CSV configuration options.

The function signature updates in the documentation correctly describe the new csvOptions parameter for flexible CSV configuration.

Also applies to: 197-203


479-479: LGTM! Removal of date format tag enables flexible date format support.

The removal of the format tag from the Date field aligns with the PR's objective of supporting different date time formats.

@@ -14,12 +14,12 @@ import (
)

func TestBopStrategy(t *testing.T) {
snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv", true)
snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv")
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Incorrect test data path needs to be updated

The test file is using an incorrect path. Based on the established pattern in other strategy tests, update the following paths:

  • Change testdata/brk-b.csv to testdata/repository/brk-b.csv
  • The path testdata/bop_strategy.csv is correct and can remain as is
🔗 Analysis chain

Verify CSV file structure and date format handling.

The removal of the boolean parameter suggests these CSV files previously had headers. Please ensure:

  1. The test data files still work correctly with the new CSV configuration
  2. Any date/time formats in the test data align with the new configurable format support

Also applies to: 22-22

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check CSV file structure and potential date formats
echo "=== brk-b.csv structure ==="
head -n 2 testdata/brk-b.csv

echo -e "\n=== bop_strategy.csv structure ==="
head -n 2 testdata/bop_strategy.csv

Length of output: 381


Script:

#!/bin/bash
# Search for CSV files and references
echo "=== Finding CSV files ==="
fd -e csv

echo -e "\n=== Searching for brk-b.csv references ==="
rg "brk-b.csv"

echo -e "\n=== Searching for bop_strategy.csv references ==="
rg "bop_strategy.csv"

echo -e "\n=== Looking for potential test data directory ==="
fd testdata

Length of output: 14920

@@ -14,12 +14,12 @@ import (
)

func TestGoldenCrossStrategy(t *testing.T) {
snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv", true)
snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv")
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add test coverage for CSV configuration options.

To ensure the new CSV configuration capabilities work as intended, consider adding test cases that cover:

  • Different date time formats
  • Files with and without headers
  • Custom CSV options

This aligns with the PR's objective of supporting different date time formats for snapshots.

Would you like me to help create comprehensive test cases that demonstrate the new CSV configuration capabilities?

Also applies to: 22-22, 39-39

@@ -15,12 +15,12 @@ import (
)

func TestNoLossStrategy(t *testing.T) {
snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv", true)
snapshots, err := helper.ReadFromCsvFile[asset.Snapshot]("testdata/brk-b.csv")
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add test cases for different date formats.

Since this PR adds support for configuring different date time formats, and this test uses asset.Snapshot which contains the Date field, consider adding test cases with different date formats to verify the new functionality.

Would you like me to help create additional test cases with different date formats?

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