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

Improve the exception message when an option within the rootpath section is missing from the user configuration file #2236

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

ehogan
Copy link
Contributor

@ehogan ehogan commented Oct 19, 2023

Description

Closes #2235

Link to documentation:


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@ehogan ehogan self-assigned this Oct 19, 2023
@ehogan ehogan added enhancement New feature or request UX User experience labels Oct 19, 2023
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #2236 (a741e19) into main (7a70d5b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2236   +/-   ##
=======================================
  Coverage   93.47%   93.48%           
=======================================
  Files         238      238           
  Lines       12927    12927           
=======================================
+ Hits        12084    12085    +1     
+ Misses        843      842    -1     
Files Coverage Δ
esmvalcore/local.py 98.71% <100.00%> (+0.32%) ⬆️

Comment on lines +411 to +412
raise KeyError(f'The "{project}" option is missing from the "rootpath" '
'section in the config-user.yml file.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pre-commit hooks made all the changes in this PR, except for this one, which is the one I made :)

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

cheers @ehogan = looking good and very useful!

with mock.patch.dict(local.CFG, cfg):
msg = rf"The \"{project}\" option is missing.*"
with pytest.raises(KeyError, match=msg):
local._get_rootpath(project)
Copy link
Contributor

Choose a reason for hiding this comment

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

well, you made these changes too, unless precommit hooks now write unit tests too (which would be fab) 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤣 🤣 🤣 I added these after I made that comment! 😝

@valeriupredoi
Copy link
Contributor

cheers @ehogan 🍺 @zklaus I reckon this can go in 2.10 but you da big honcho 🤠

@bouweandela bouweandela merged commit 6e61590 into main Nov 13, 2023
3 checks passed
@bouweandela bouweandela deleted the 2235_improve_exception_message branch November 13, 2023 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request UX User experience
Projects
None yet
3 participants