Support one-file open_dataset

[rajeeja]

Closes #345 by allowing ux.open_dataset(file) for combined grid-and-data files.

[Copilot]

Pull request overview

Adds support for opening combined grid-and-data files via a single-argument ux.open_dataset(file) call, aligning with Issue #345’s request to handle datasets stored in one file.

Changes:

• Made filename_or_obj optional in uxarray.core.api.open_dataset, defaulting to the grid file path when omitted.
• Updated open_dataset docstring to document and exemplify the one-file usage.
• Added a unit test covering the new one-argument open_dataset behavior.

Reviewed changes

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

File	Description
uxarray/core/api.py	Makes filename_or_obj optional and adds logic/docs for single-file grid+data opening.
test/core/test_api.py	Adds a regression test for calling ux.open_dataset() with a single argument.

---

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

[erogluorhan]

Thanks a lot for taking this one. Please find my comments below

[Sevans711]

Changes mostly make sense, code looks reasonable, tests cover new cases. Tested locally and behavior seems reasonable. I read through the older comments in the thread on this PR as well. A few lingering thoughts / comments:

I was surprised to discover that this enables ux.open_dataset(grid_file) regardless of whether the grid file also contains "non-grid" data. Added a comment about this, I think it could be cleared up with just a slight change to the docstring.

I was confused by the "Directory-based gridfiles" crash. Nothing in the docstring suggested that the inputs could be directories in the first place, so I didn't realize directory-based gridfiles even existed at first. Suggested rephrasing. Also, I separately may be trying to add support for ux.open_dataset(directory) in some simple cases soon (see #1555). Clarifying this now will help with that.

Finally, I committed a minor change to error message when failing due to typo like ux.open_dataset(data_file). Before the error was confusing; now it is clearer that the issue is being unable to parse uxgrid information from the inputs.

[Sevans711]

I would like to clarify the docs a bit:

• grid_filename_or_obj docstring could start with something like "Grid information for the UxDataset" to help quickly clarify this is always the grid info.
• grid_filename_or_obj docstring should clarify it can be a path to a directory containing grid files (but only if filename_or_obj is provided).
• filename_or_obj should clarify to add something like "Or, it can be omitted to treat the grid as data, for any grid file"

The third point I am saying because these changes enable you to do open_dataset(grid_file) for any grid file, regardless of whether it is "combined" with data or not.

[Sevans711]

What is a "Directory-based grid?" This was my first thought. I explored the code a bit more to see there is one special case of directory-based grids.

I would maybe prefer an error like: "open_dataset(directory) is currently not supported. Consider supplying a path to a grid file instead. In special cases (e.g. for FESOM2 ASCII Dataset) directory-based grids can be recognized, but only if also providing a data file, such as via open_dataset(directory, data_file)."

Although… actually I am not really sure why we can't extend "open a directory-based-grid as a UxDataset" the same way we would be supporting "open a single-file grid as a UxDataset". How confusing is the downstream xarray crash?

[rajeeja]

@Sevans711 Docstrings clarified (grid-vs-data roles, single-arg reads non-grid vars as data) and the directory error message rewritten. Also noted partial / non-UGRID / non-Earth grids can be opened, features degrading as needed.

[Sevans711]

@rajeeja Thank you for adding those clarifications! One more thought on the new wording, and then I think the rest looks good and I would be happy to approve. Current wording:

If omitted,grid_filename_or_obj is also used as the data source, allowing a combined grid-and-data file or xarray.Dataset to be opened with a single argument. In this single-argument form the file is used both to build the grid and to load the data; any non-grid variables it contains are read as data.

I would change this to something like (bold here to show changes):

If omitted,grid_filename_or_obj is also used as the data source, allowing a combined grid-and-data file or xarray.Dataset to be opened with a single argument. In this single-argument form the file is used to build the grid and also treated as data; all variables it contains, including grid variables, will be included as data_vars in the result.

This clarifies the unexpected behavior I was noticing. E.g., result['node_lon'] exists when building the dataset this way, in addition to things like result['bottomDepth'] or other physical data. This behavior seems reasonable enough, it just wasn't my initial intuition about what would happen, so clarifying the docs should be sufficient.