Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modernize testing, load configurations more dynamically, use pooch #120

Closed
wants to merge 9 commits into from

Conversation

Zeitsperre
Copy link
Contributor

@Zeitsperre Zeitsperre commented Oct 28, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes issue #xyz
  • Tests for the changes have been added (for bug fixes / features)
  • Documentation has been added / updated (for bug fixes / features)
  • HISTORY.rst has been updated (with summary of main changes)
  • I have added my relevant user information to AUTHORS.md

What kind of change does this PR introduce?:

  • Rewrites a large portion of the pytest suite to prevent modified configurations from being propagated to other tests
  • Modernizes the packaging to use statically-declared metadata pyproject.toml with the flit-core backend.
  • Drops Python3.8
  • Adds a new reload_config() function that can be used to forcibly overwrite a roocs-like configuration on-the-fly
  • gitpython has been removed in favour of pooch and registry files.
  • Fixes the documentation by addressing all existing warnings, and adds the modules to the table of contents.

Side effects:

  • roocs_utils now supports pytest-xdist (disabled by default).

Does this PR introduce a breaking change?:

Yes and no. The configuration is essentially the same but is now loaded similarly to what is seen in clisops:

  • A project is first defined.
  • the roocs_utils.get_config() function is called.
  • the "environment" env var is parsed to add projects to the namespace.

The pytest suite was modifying the CONFIG on load to include some paths to local testing data that was copied using gitpython. This has been replaced with pooch registries that create testing data folders under XDG_CACHE (or similar), and verified using sha256sum and git tags/branches for version control.

This new system is almost directly copied from clisops, with the intention of making as minimal changes as possible so that the projects can be easily merged.

I also added a reload_config() function that is used specifically in a handful of tests that require the modified ROOCS_CONFIG file. This works by removing the existing _CONFIG (found under config.py) and recreating it. Since the CONFIG is initialized on the load of roocs_utils (and submodules then get that object from the top-level), if you want to set temporary configurations (such as for testing), you need to:

  • Forcibly remove CONFIG in the affected submodule.
  • Set the env vars for the running operation (done using pytest.monkeypatch.setenv fixture).
  • Run the reload_config(), setting the affected submodule's CONFIG to the result.
  • Then revert the process to set things to normal (using pytest.monkeypatch.delenv and reload_config())

Other information:

I wish there was a way to simplify this further, but this needs to be moved into clisops first (#107).

@Zeitsperre Zeitsperre self-assigned this Oct 28, 2024
@Zeitsperre Zeitsperre marked this pull request as ready for review October 28, 2024 20:49
@cehbrecht
Copy link
Contributor

@Zeitsperre do you want to move this PR directly into clisops? So, we don't merge it?

@Zeitsperre
Copy link
Contributor Author

@cehbrecht Yes, I think that would be much simpler. It would be helpful to know which modules are no longer required to be ported now that Martin's changes are in CLISOPS (is it just xarray_utils?).

I'll have time to get back to this soon, but my intention here (reflecting what we talked about) was to port my changes directly into CLISOPS.

@cehbrecht
Copy link
Contributor

Yes, I think that would be much simpler. It would be helpful to know which modules are no longer required to be ported now that Martin's changes are in CLISOPS (is it just xarray_utils?).

Yes ... I think so ... Martin integrated xarray_utils already in clisops.

@Zeitsperre
Copy link
Contributor Author

The work here has been rendered obsolete. Closing.

@Zeitsperre Zeitsperre closed this Dec 12, 2024
@Zeitsperre Zeitsperre deleted the modernize branch December 12, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants