OceanParcels / Parcels

Main code for Parcels (Probably A Really Computationally Efficient Lagrangian Simulator)
https://www.oceanparcels.org
MIT License
289 stars 129 forks source link

Add FieldSet.from_a_grid_dataset() method #1642

Closed erikvansebille closed 1 month ago

erikvansebille commented 1 month ago

Parcels has specific methods for loading B-grid and C-grid datasets, but not for A-grids. This is because A-grid (linear) interpolation is default, so FieldSet.from_netcdf() automatically assumes an A-grid.

However, for consistency and to help new users, it might be nice to add an explicit FieldSet.from_a_grid_dataset() method to Parcels. This method would then simply call from_netcdf().

Note that while this is done, it might also be good to update a few mentions of from_bgrid() and from_cgrid() in the docstrings to the correct from_b_grid_dataset() and from_c_grid_dataset()

KOMPALALOKESH commented 1 month ago

Hi @erikvansebille , I'd like to work on this enhancement. Could you please assign this issue to me?

Thank you!

erikvansebille commented 1 month ago

Thanks @KOMPALALOKESH, for wanting to pick this up! I've assigned this Issue to you.

I think the key starting point is to make a new method in parcels/fieldset.py that returns a cls.from_netcdf(), a bit like how fieldset.from_b_grid() is structured but then without much (any?) adjustments.

It would also be nice to add a unit test to tests/test_fieldset.py.

Good luck, and let us know if you need any help!

VeckoTheGecko commented 1 month ago

@KOMPALALOKESH if you're interested and want more than this issue, "Enable pyupgrade on ruff linting" from #1620 is well defined and might be interesting from a technical standpoint if you haven't worked with Ruff before. Its a nice Python tool, and modern Python is consolidating towards it for handling QAQC.

KOMPALALOKESH commented 1 month ago

@erikvansebille , This PR Link introduces the FieldSet.from_a_grid_dataset() method in "parcels/fieldset.py" to handle A-grid datasets. A corresponding unit test has been added in "tests/test_fieldset.py" to validate this functionality.

Please review the changes and the test case. If everything is in order, kindly merge.