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

Provenance hints to user_note and user_error and error handling for missing coupling direction #1277

Merged
merged 6 commits into from
Feb 3, 2025

Conversation

mandresm
Copy link
Contributor

@mandresm mandresm commented Jan 21, 2025

  • Error handling for Possible naming problem in esm_tools. #1268
  • adds get_first_provenance method to more easily use the first occurrence of provenance in a dictionary or list to report suggestions in the error messages
  • adds hint functionality to user_error and user_note: if you add a list of hints as input to one of this methods, you can add <HINT_#> placeholders to your message for user_note and user_error. These place holders will be substituted using the hint's properties you define for each hint.

For #1268 it generates this user error now:
Screenshot 2025-01-21 at 19 23 12

Closes #1268

@mandresm mandresm requested a review from pgierz January 21, 2025 16:37
@mandresm
Copy link
Contributor Author

This can still be improved, for example, by making an Hint object and an UserNote object. However, I think improving it would only make sense if we refactor the whole user_note thing, move it to it's own module, and including the loguru logger into it. WDYT @pgierz?

@mandresm mandresm changed the title add get_first_provenance and error handling for #1268 Provenance hints to user_note and user_error and a fix to #1268 Jan 21, 2025
@mandresm mandresm changed the title Provenance hints to user_note and user_error and a fix to #1268 Provenance hints to user_note and user_error and a fix to 1268 Jan 21, 2025
@mandresm mandresm changed the title Provenance hints to user_note and user_error and a fix to 1268 Provenance hints to user_note and user_error and a fix to #1268 Jan 21, 2025
@mandresm mandresm changed the title Provenance hints to user_note and user_error and a fix to #1268 Provenance hints to user_note and user_error and error handling for missing coupling direction Jan 21, 2025
@pgierz
Copy link
Member

pgierz commented Jan 22, 2025

Yes, plus 100000000 points. Having those be in esm_parser is not where they belong. Suggestion: put it in the esm_tools module directly, which is where we should keep things that are needed in all of the sub packages.

from loguru import logger
# Alternatively: from esm_tools.logging import logger (see implementation in pymorize)

class Hint
   def __init__(self, message):
       self.message = message

   def error(self, message=None):
        if message is None:
            message = self.message
        logger.error(message)
   # Same with info, debug, critical, etc

...something like that. Maybe with static methods so you don't need to initialise the object.

…r_handling.py module). Black error_handling.py. isort all .py files containing changes
@mandresm
Copy link
Contributor Author

Suggestion: put it in the esm_tools module directly, which is where we should keep things that are needed in all of the sub packages.

done!

Copy link
Member

@pgierz pgierz left a comment

Choose a reason for hiding this comment

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

I have one naming suggestion, otherwise this is good to go.

src/esm_parser/provenance.py Outdated Show resolved Hide resolved
src/esm_parser/provenance.py Outdated Show resolved Hide resolved
@mandresm mandresm requested a review from pgierz January 30, 2025 07:11
@mandresm
Copy link
Contributor Author

mandresm commented Feb 3, 2025

#bump

@mandresm mandresm merged commit 70acca9 into release Feb 3, 2025
13 of 16 checks passed
@mandresm mandresm deleted the feat/get_first_provenance branch February 3, 2025 09:06
@mandresm mandresm restored the feat/get_first_provenance branch February 3, 2025 09:06
@mandresm
Copy link
Contributor Author

mandresm commented Feb 3, 2025

#bump

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.

Possible naming problem in esm_tools.
2 participants