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

Use platformdirs #323

Merged
merged 9 commits into from
Feb 15, 2024
Merged

Use platformdirs #323

merged 9 commits into from
Feb 15, 2024

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Feb 9, 2024

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • 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?:

  • Uses platformdirs to set a default platform-dependent location in the event that one is not provided for local_weights_dir.
  • platformdirs now handles the caching location for testing data.
  • require_module now handles supported module version information.
  • Fixed the version pinning and comparison code for xarray (2023.032023.3)

Does this PR introduce a breaking change?:

Yes. The default location hard-set in the roocs.ini configuration has been removed.

Additionally, the .clisops_testing_data folder can be removed from your $HOME folder. This is now saved to $HOME/.cache/clisops on *nix.

Other information:

On Linux, this looks like:

from clisops.core.regrid import CONFIG

CONFIG["clisops:grid_weights"]["local_weights_dir"]
>>>'/home/my_user/.local/share/clisops/weights_dir'

@coveralls
Copy link

coveralls commented Feb 9, 2024

Pull Request Test Coverage Report for Build 7898546444

Details

  • -11 of 29 (62.07%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 72.471%

Changes Missing Coverage Covered Lines Changed/Added Lines %
clisops/core/regrid.py 14 16 87.5%
clisops/utils/common.py 2 11 18.18%
Files with Coverage Reduction New Missed Lines %
clisops/utils/common.py 1 68.63%
Totals Coverage Status
Change from base Build 7890990318: -0.2%
Covered Lines: 1777
Relevant Lines: 2452

💛 - Coveralls

@cehbrecht
Copy link
Collaborator

@Zeitsperre @sol1105 when using platformdirs could we skip the roocs.ini config for weights_dir?

@Zeitsperre
Copy link
Collaborator Author

@cehbrecht I suppose you technically could. I've set it up so that if the option is not provided, it'll use a cached location, but if you'd rather remove this option entirely, you should be able to work with locations provided uniquely by platformdirs.

Copy link
Contributor

@sol1105 sol1105 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks great, thank you.

In the regrid notebook of the documentation, I used the explicit path instead of the CONFIG settings. I will update that
https://clisops--323.org.readthedocs.build/en/323/notebooks/regrid.html#Local-weights-cache

In general I wonder, whether the ./binder/environment.yml and ./docs/environment.yml are too extensive. Should they only include packages additionally required with regards to the basic ./environment.yml?

@Zeitsperre Zeitsperre merged commit c04ce3d into master Feb 15, 2024
11 checks passed
@Zeitsperre Zeitsperre deleted the fix-tmpfile-location branch February 15, 2024 19:38
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.

4 participants