Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors RBM dataset handling by consolidating NetCDF (.nc) support into the existing RBMDataSet implementation, removing the separate NetCDF dataset class and dataset manager.
Changes:
- Add NetCDF recursive variable reading and NetCDF-specific variable loading/mapping to
RBMDataSet. - Extend
preferred_extensionto support"nc"and adjust file path/name stem logic for NetCDF directory conventions. - Restructure tests by removing
RBMNcDataSet/manager tests and adding NetCDF-focused tests to the mainRBMDataSettest suite.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
swvo/io/RBMDataSet/RBMDataSet.py |
Adds NetCDF support (reader + loader dispatch), extends preferred_extension, and adjusts computed variable caching/loaded-variable tracking. |
swvo/io/RBMDataSet/__init__.py |
Removes exports for RBMDataSetManager and RBMNcDataSet, leaving only RBMDataSet as the main entry point. |
swvo/io/RBMDataSet/RBMNcDataSet.py |
Deleted (NetCDF functionality moved into RBMDataSet). |
swvo/io/RBMDataSet/RBMDataSetManager.py |
Deleted (manager no longer available/exported). |
tests/io/RBMDataSet/test_RBMDataset.py |
Adds NetCDF-mode tests and new coverage for computed variables being tracked by get_loaded_variables(). |
tests/io/RBMDataSet/test_RBMNcDataset.py |
Deleted (superseded by NetCDF tests in test_RBMDataset.py). |
tests/io/RBMDataSet/test_RBMDatasetManager.py |
Deleted (manager removed). |
Comments suppressed due to low confidence (1)
swvo/io/RBMDataSet/init.py:26
- Removing
RBMDataSetManager(andRBMNcDataSet) from the public package exports is a breaking change and currently leaves internal code referencing these names (e.g.swvo/io/RBMDataSet/scripts/create_RBSP_line_data.pystill importsRBMDataSetManager, and some modules referenceRBMNcDataSetunderTYPE_CHECKING). Either keep/backfill these symbols (possibly as deprecated aliases) or update the remaining internal imports/type hints accordingly so the package imports and type-checking don’t break.
from swvo.io.RBMDataSet.interp_functions import TargetType as TargetType
from swvo.io.RBMDataSet.scripts.create_RBSP_line_data import create_RBSP_line_data as create_RBSP_line_data
from swvo.io.RBMDataSet.RBMDataSet import RBMDataSet as RBMDataSet
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request extends the
RBMDataSetclass to add comprehensive support for loading and managing RBM data from NetCDF (.nc) files, in addition to the existing.matand.pickleformats. The changes introduce format-specific loading logic, variable mapping for NetCDF files, and update the class interface and documentation accordingly. The most important changes are grouped below:NetCDF File Support and Variable Loading:
_read_all_datasets_netcdfhelper function to recursively read all variables (including those in groups) from NetCDF-4 files and return them in a dictionary keyed by their full hierarchical path._load_variable_netcdfand_get_rbm_name_for_ncmethods to handle loading variables from NetCDF files and mapping NetCDF variable names to internal RBM variable names. This includes logic for time trimming, computed variables, and attribute assignment. (Fa091418L445R534, Fa091418L534R621)_load_variablemethod to dispatch to either NetCDF or.mat/.pickleloading logic based on the selected file extension. (Fa091418L445R534)Class Interface and Initialization:
preferred_extensionparameter and related logic to accept"nc"as a valid option, with validation and storage in the class. [1] [2] [3]variable_lutmapping NetCDF variable paths to internal variable names, including handling of the magnetic field model (mfm).File Path and Name Handling:
_create_file_path_stemand_create_file_name_stemto generate the correct file path and name patterns for NetCDF files, which differ from.matand.pickleconventions. [1] [2]Documentation and Usability:
Other Improvements:
PandInvVto cache results and handle empty values more robustly.These changes make the
RBMDataSetclass more flexible and robust, allowing it to seamlessly handle multiple data formats and improving its usability for a wider range of datasets.