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

Refactor config default search path retrieval #19

Merged
merged 1 commit into from Feb 4, 2022

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Feb 4, 2022

Port of dask/dask#8573 to donfig.

Authored by @jrbourbeau in the original dask PR. This removes a long deprecated path from the search list. A lot of the additional refactoring done by James in his PR is not needed here as donfig is already required to use a separate Config object for each new instance. So for this PR's tests the Config.paths property is considered public while it may not be considered that in dask. Lastly, I did not include James' last test:

def test_default_search_paths():
    # Ensure _get_paths() is used for default paths
    assert dask.config.paths == _get_paths()

as I'm not sure how I would structure donfig to do this. I could extract the path logic to another function, but it would depend on name of the config and the environment variable. At this time this logic is a little too interconnected. For now I'm leaving it.

Co-authored-by: David Hoese <david.hoese@ssec.wisc.edu>
Co-authored-by: James Bourbeau <jrbourbeau@users.noreply.github.com>
@djhoese djhoese added the enhancement New feature or request label Feb 4, 2022
@codecov
Copy link

codecov bot commented Feb 4, 2022

Codecov Report

Merging #19 (322c802) into main (adbb8d1) will increase coverage by 0.60%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #19      +/-   ##
==========================================
+ Coverage   93.81%   94.41%   +0.60%     
==========================================
  Files           4        4              
  Lines         582      609      +27     
==========================================
+ Hits          546      575      +29     
+ Misses         36       34       -2     
Flag Coverage Δ
unittests 94.41% <100.00%> (+0.60%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
donfig/config_obj.py 91.05% <ø> (+0.81%) ⬆️
donfig/tests/test_config.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update adbb8d1...322c802. Read the comment docs.

@djhoese djhoese merged commit 5000416 into pytroll:main Feb 4, 2022
@djhoese djhoese deleted the feature-config-search-order branch February 4, 2022 21:32
@jrbourbeau
Copy link
Contributor

🎉

FYI there was one additional test-related change in dask/dask#8644 which was added after dask/dask#8573

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants