added ability to read SEGY file version 0.0 #483
Conversation
WalkthroughA new class Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (15)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
dascore/io/segy/core.py (1)
90-94: Consider refactoring the SEGY version class hierarchy.The current design has all version classes inheriting from
SegyV1_0, including both older (V0_0) and newer (V2_x) versions. This might make it difficult to implement version-specific behavior in the future.Consider introducing a base
SegyBaseclass that contains common functionality, allowing each version to implement its specific behavior independently.Would you like me to propose a refactored class hierarchy that better supports version-specific implementations?
🧰 Tools
🪛 Ruff (0.8.2)
91-91: No whitespaces allowed surrounding docstring text
Trim surrounding whitespace
(D210)
91-91: Line too long (95 > 88)
(E501)
🪛 GitHub Actions: LintCode
[error] 91-91: Line too long (94 > 88 characters)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dascore/io/segy/core.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
dascore/io/segy/core.py
91-91: No whitespaces allowed surrounding docstring text
Trim surrounding whitespace
(D210)
91-91: Line too long (95 > 88)
(E501)
🪛 GitHub Actions: LintCode
dascore/io/segy/core.py
[error] 91-91: Line too long (94 > 88 characters)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: test_code (windows-latest, 3.12)
- GitHub Check: test_code (windows-latest, 3.11)
- GitHub Check: test_code (windows-latest, 3.10)
- GitHub Check: test_code (macos-latest, 3.12)
- GitHub Check: test_code_min_deps (windows-latest, 3.13)
- GitHub Check: test_code (macos-latest, 3.11)
- GitHub Check: test_code_min_deps (windows-latest, 3.12)
- GitHub Check: test_code (macos-latest, 3.10)
- GitHub Check: test_code_min_deps (macos-latest, 3.13)
- GitHub Check: test_code (ubuntu-latest, 3.12)
- GitHub Check: test_code_min_deps (macos-latest, 3.12)
- GitHub Check: test_code (ubuntu-latest, 3.11)
- GitHub Check: test_code_min_deps (ubuntu-latest, 3.13)
- GitHub Check: test_code (ubuntu-latest, 3.10)
- GitHub Check: test_code_min_deps (ubuntu-latest, 3.12)
🔇 Additional comments (1)
dascore/io/segy/core.py (1)
90-94: Verify version detection logic.Since this class handles both version 0.0 and unset versions, we should verify the version detection logic in
_get_segy_version.Let's check the version detection implementation:
✅ Verification successful
Version detection implementation is correct and robust
The version detection logic in
_get_segy_versionproperly handles both version 0.0 and unset versions by:
- Reading version numbers from the standard SEGY binary header (bytes 300-302)
- Validating major version numbers against allowed values (0,1,2,3)
- Falling back to SegyV0_0 for both version 0.0 and invalid/unset versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how version detection is implemented # Search for version detection function ast-grep --pattern 'def _get_segy_version($$$)' # Search for any version-related constants or patterns rg -A 5 'version.*0\.0|unset.*version' --type pyLength of output: 2806
Script:
#!/bin/bash # Search for SegyV1_0 class and its methods ast-grep --pattern 'class SegyV1_0($$$)' # Look for version-related methods in SEGY files rg -A 10 'def.*version' dascore/io/segy/ # Search for format reading code rg -A 5 'read.*format|parse.*format|detect.*format' dascore/io/segy/Length of output: 1710
Script:
#!/bin/bash # Get the complete implementation of _get_segy_version rg -A 30 '^def _get_segy_version' dascore/io/segy/utils.pyLength of output: 1281
🧰 Tools
🪛 Ruff (0.8.2)
91-91: No whitespaces allowed surrounding docstring text
Trim surrounding whitespace
(D210)
91-91: Line too long (95 > 88)
(E501)
🪛 GitHub Actions: LintCode
[error] 91-91: Line too long (94 > 88 characters)
dascore/io/segy/core.py
Outdated
| class SegyV0_0(SegyV1_0): # noqa | ||
| """An IO class supporting version 0.0 of the SEGY format. Or if the version is not set. """ | ||
|
|
||
| version = "0.0" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix docstring formatting and line length.
The docstring has formatting issues and exceeds the line length limit. Additionally, consider documenting how unset versions are detected.
Apply this diff to fix the formatting:
-class SegyV0_0(SegyV1_0): # noqa
- """An IO class supporting version 0.0 of the SEGY format. Or if the version is not set. """
+class SegyV0_0(SegyV1_0): # noqa
+ """An IO class supporting version 0.0 of the SEGY format.
+
+ This class handles both explicit version 0.0 and cases where the version is not set.
+ """📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class SegyV0_0(SegyV1_0): # noqa | |
| """An IO class supporting version 0.0 of the SEGY format. Or if the version is not set. """ | |
| version = "0.0" | |
| class SegyV0_0(SegyV1_0): # noqa | |
| """An IO class supporting version 0.0 of the SEGY format. | |
| This class handles both explicit version 0.0 and cases where the version is not set. | |
| """ | |
| version = "0.0" |
🧰 Tools
🪛 Ruff (0.8.2)
91-91: No whitespaces allowed surrounding docstring text
Trim surrounding whitespace
(D210)
91-91: Line too long (95 > 88)
(E501)
🪛 GitHub Actions: LintCode
[error] 91-91: Line too long (94 > 88 characters)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #483 +/- ##
=======================================
Coverage 99.85% 99.85%
=======================================
Files 118 118
Lines 9700 9702 +2
=======================================
+ Hits 9686 9688 +2
Misses 14 14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
Added ability to read a segy file if version is set to 0.0
Tested on file from:
https://ddfe.curtin.edu.au/7h0e-d392/2020_GeoLab_WVSP_Geophone_wgm.sgy
Checklist
I have (if applicable):
Summary by CodeRabbit