Refactor PackageContainer composition over inheritance

[deltamarnix]

In preparation of larger refactor works in flopy, we discussed with @wpbonelli and @mjreno that it would be beneficial for readability to change the PackageContainer to a composition pattern, rather than inheritance. Only 7 functions need to be duplicated in MFSimulationBase, MFPackage, and MFModel as pass-through functions. There should be no compatibility issues for the users, as the signature hasn't changed.

My doubts

There are some public API functions that are now part of MFPackage, MFModel, MFSimulationBase of which I doubt if we should keep them. They don't seem to be called in any tests and I wonder if they should be available. The function that seems reasonable to keep is therefore only get_package, and maybe only for MFSimulationBase.

• package_key_dict
• package_dict
• package_names
• package_type_dict
• package_name_dict
• package_filename_dict

I changed MFPackage._simulation_data to MFPackage.simulation_data to be consistent with MFSimulationBase and MFModel. Also because it is used by output_util.MF6Output (line 115).

Breaking changes

Static variables modflow_packages, packages_by_abbr, modflow_models, and models_by_type should be called via PackageContainer.modflow_packages etc. If these variables are part of the public API, the users might need to change some code to no longer call MFPackage.modflow_packages or the like.

It is possible to make this a non-breaking change by introducing a meta-class, but if possible the user does not need these attributes, I would try to not take this route.

class PackageContainerProperties(type):
    @property
    def modflow_packages(cls):
        return PackageContainer.modflow_packages

class MFPackage(metaclass=PackageContainerProperties):
    ...

[wpbonelli]

My sense after the discussion today is that where we are unclear if an API is meant to be user-facing, we can keep it intact and add a deprecation warning, which will (we hope) give us an idea whether anyone is using it — if not, after some period (how long?) it is fair game for pre-v4 refactoring?

[deltamarnix]

@wpbonelli Should I also add deprecation warnings for the class variables modflow_packages etc. as mentioned in the top comment under "breaking changes"? That would mean I will have to implement the meta class just for deprecation purposes.

[langevin-usgs]

I've never used any of those variables you listed under "breaking changes". If it requires additional work to issue a deprecation warning, then my opinion is that you can just rename/remove them, especially if none of the tests or examples use them.

[wpbonelli]

can confirm none of

• modflow_packages
• packages_by_abbr
• modflow_models
• models_by_type

are used in flopy tests, mf6 tests, or mf6 examples

[deltamarnix]

@wpbonelli I have remove usages in the tests for package_type_dict. Instead users should be using get_package() if they want a package to work with.
I have also added deprecation warnings.

[wpbonelli]

looks good just a few things