Standardizing module import method for pyFV3 #5

fmalatino · 2024-02-13T20:10:10Z

Description
Adding __init__ files to standardize import method of modules

How Has This Been Tested?
Tested via lint and currently implemented translate tests
OS: RHEL 8.9 Ootpa
Built conda environment using workflow in github action configurations

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules
New check tests, if applicable, are included
Targeted model if this changed was triggered by a model need/shortcoming

… modules of pyFV3

FlorianDeconinck

Good stuff. I'd drop the test one, what do you think?

FlorianDeconinck · 2024-02-14T15:07:59Z

pyFV3/stencils/__init__.py

@@ -0,0 +1,28 @@
+from .a2b_ord4 import AGrid2BGridFourthOrder


Poking @oelbert : an idea here. If we put down some one/two liner to explicitly describe each Module scientific purpose it may look nice in the docs. To be tested.

probably a good idea, something like?

""" Converts a field from the Arakawa A grid to B grid """

Yes, we have to generate the docs and see how it shows

FlorianDeconinck · 2024-02-14T15:08:04Z

tests/mpi/__init__.py

Do we need to package the sub-directories of the test? Those are more end-user files, with no real API to publicize

I think it's unnecessary, but if MSD wants to package the tests I won't object too strongly

I have no immediate need or reason to package them. I have been adding __init__ where they aren't for anything that looks like it can/might have a reason to have one.

FlorianDeconinck · 2024-02-14T15:08:42Z

pyFV3/initialization/__init__.py

@@ -1 +1,3 @@
 from .analytic_init import init_analytic_state


Like in stencils this might be a good place for some cursory scientific description

xyuan

this looks good to me

FlorianDeconinck · 2024-02-16T16:33:31Z

Please merge in develop instead of main

oelbert

There are a couple of docstrings that should be clarified, and there's a lot more exposed in the pyFV3 init than I thought we'd agreed on?

oelbert · 2024-02-20T15:50:14Z

pyFV3/__init__.py

+from .stencils import (
+    AcousticDynamics,
+    AdjustNegativeTracerMixingRatio,
+    CGridShallowWaterDynamics,
+    DGrid2AGrid2CGridVectors,
+    DivergenceDamping,
+    DryConvectiveAdjustment,
+    DynamicalCore,
+    FiniteVolumeFluxPrep,
+    FiniteVolumeTransport,
+    HyperdiffusionDamping,
+    LagrangianToEulerian,
+    NonhydrostaticVerticalSolver,
+    NonhydrostaticVerticalSolverCGrid,
+    PK3Halo,
+    RayleighDamping,
+    SatAdjust3d,
+    TracerAdvection,
+    UpdateGeopotentialHeightOnCGrid,
+    UpdateHeightOnDGrid,
+    XPiecewiseParabolic,
+    YPiecewiseParabolic,
+)
+from .testing import (
+    TranslateDycoreFortranData2Py,
+    TranslateDynCore,
+    TranslateFVDynamics,
+)
+from .wrappers import GeosDycoreWrapper


I thought we weren't going to expose the stencil classes or test code or wrapper at this level?

testing definitely not.
stencils we can discuss. DynamicalCore needs to be exposed or else Pace can't do anything really, the other could be hidden if we consider them not re-usable.
wrappers needs to be exposed, it's public API.

pyFV3/testing/__init__.py

Co-authored-by: Oliver Elbert <[email protected]>

… level

…to feature/init_standard

oelbert

Minor note about exposing wrappers, but not blocking

oelbert · 2024-02-20T18:11:11Z

pyFV3/__init__.py

-from .stencils.fv_subgridz import DryConvectiveAdjustment
-from .wrappers.geos_wrapper import GeosDycoreWrapper
+from .stencils import DryConvectiveAdjustment, DynamicalCore
+from .wrappers import GeosDycoreWrapper


I'm not totally convinced we want to expose the wrappers here, but it's probably ok with just the GEOS wrapper, though I may want to revisit if it starts getting unwieldy

Fair opposition.

Technically a Wrapper is so bespoke it's a bit of a stretch for me to call it a "public" API. I wouldn't object to removing it.

I might in the future take it off pyFV3 entirely and move it to GEOS itself

oelbert

👍

fmalatino added 2 commits February 12, 2024 14:46

Updated __init__ files for the initialization, stencils, and wrappers…

3107bcd

… modules of pyFV3

Adding __init__ files for all modules

c3ac99c

fmalatino requested review from xyuan, FlorianDeconinck, oelbert, bensonr and mlee03 February 14, 2024 15:01

FlorianDeconinck reviewed Feb 14, 2024

View reviewed changes

fmalatino changed the title ~~Standardizing module import method~~ Standardizing module import method for pyFV3 Feb 14, 2024

xyuan previously approved these changes Feb 14, 2024

View reviewed changes

Adding doc strings to import modules

b7022f7

fmalatino dismissed xyuan’s stale review via b7022f7 February 15, 2024 19:36

fmalatino requested a review from FlorianDeconinck February 15, 2024 19:39

fmalatino marked this pull request as draft February 15, 2024 20:46

Amended import methods of tests/

da1b847

fmalatino changed the base branch from main to develop February 16, 2024 17:27

fmalatino added 2 commits February 16, 2024 14:27

More tidying up

0d4a6e9

Liniting

e221b8a

fmalatino marked this pull request as ready for review February 20, 2024 13:56

oelbert requested changes Feb 20, 2024

View reviewed changes

fmalatino and others added 3 commits February 20, 2024 12:18

Update pyFV3/testing/__init__.py

25d9b4a

Co-authored-by: Oliver Elbert <[email protected]>

Removing testing and majority of stencils from being exposed at pyFV3…

29eaa78

… level

Merge branch 'feature/init_standard' of github.com:NOAA-GFDL/PyFV3 in…

655cc01

…to feature/init_standard

fmalatino requested a review from oelbert February 20, 2024 17:51

oelbert previously approved these changes Feb 20, 2024

View reviewed changes

Unexposing wrapper from pyFV3 level

6190a40

fmalatino dismissed oelbert’s stale review via 6190a40 February 20, 2024 18:21

fmalatino requested a review from oelbert February 20, 2024 18:21

oelbert approved these changes Feb 20, 2024

View reviewed changes

fmalatino merged commit 1deb6c6 into develop Feb 21, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standardizing module import method for pyFV3 #5

Standardizing module import method for pyFV3 #5

fmalatino commented Feb 13, 2024 •

edited

Loading

FlorianDeconinck left a comment

FlorianDeconinck Feb 14, 2024

oelbert Feb 14, 2024

FlorianDeconinck Feb 14, 2024

FlorianDeconinck Feb 14, 2024

oelbert Feb 14, 2024

fmalatino Feb 14, 2024

FlorianDeconinck Feb 14, 2024

xyuan left a comment

FlorianDeconinck commented Feb 16, 2024

oelbert left a comment

oelbert Feb 20, 2024

FlorianDeconinck Feb 20, 2024

oelbert left a comment

oelbert Feb 20, 2024

FlorianDeconinck Feb 20, 2024

oelbert left a comment

		@@ -0,0 +1,28 @@
		from .a2b_ord4 import AGrid2BGridFourthOrder

Standardizing module import method for pyFV3 #5

Standardizing module import method for pyFV3 #5

Conversation

fmalatino commented Feb 13, 2024 • edited Loading

FlorianDeconinck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xyuan left a comment

Choose a reason for hiding this comment

FlorianDeconinck commented Feb 16, 2024

oelbert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oelbert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oelbert left a comment

Choose a reason for hiding this comment

fmalatino commented Feb 13, 2024 •

edited

Loading