(Closes #3274) move access mapping from config file to constants #3278
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3278 +/- ##
==========================================
- Coverage 99.95% 99.95% -0.01%
==========================================
Files 376 376
Lines 53485 53462 -23
==========================================
- Hits 53463 53440 -23
Misses 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This is ready for a first look now from @hiker, @sergisiso or @LonelyCat124. I've fired off the IT in the meantime. |
| "PSyclone enabled keep_comments." | ||
| in caplog.record_tuples[0][2]) | ||
| output, _ = capsys.readouterr() | ||
| success = False |
There was a problem hiding this comment.
There is nothing wrong with this code but note that in other places we have been using the pattern:
with caplog.at_level(logging.WARNING):
assert "text" in caplog.text
which I think it's slightly clearer.
| assert ("' doesn't end with a recognised " | ||
| "file extension. Assuming free form." in | ||
| caplog.record_tuples[0][2]) | ||
| success = False |
| Config._HAS_CONFIG_BEEN_INITIALISED = True | ||
|
|
||
| def api_conf(self, api=None): | ||
| def api_conf(self, api: str = None): |
There was a problem hiding this comment.
= "" if the type is just str?
| Getter for the object holding API-specific configuration options. | ||
| :param str api: Optional, the API for which configuration details are | ||
| :param api: Optional, the API for which configuration details are |
There was a problem hiding this comment.
Do we still have the concept of default API? I see you modified the rtype/raises saying it errors if there the api is not valid.
Grepping "default API" has 4 hits, should we edit those?
| :rtype: Union[:py:class:`psyclone.configuration.LFRicConfig`, | ||
| :py:class:`psyclone.configuration.GOceanConfig`] |
There was a problem hiding this comment.
Move type to the signature?
| # Now create the reverse lookup (for better error messages): | ||
| self._reverse_access_mapping = {v: k for k, v in | ||
| self._access_mapping.items()} | ||
| logger.warn( |
There was a problem hiding this comment.
I get a "The function warn is deprecated: Deprecated since Python 3.3. Use Logger.warning() instead."
| # dictionary. The input is in the format: key1:value1, | ||
| # key2=value2, ... | ||
| ''' | ||
| def __init__(self, section: SectionProxy): |
| gh_write) to the AccessType (e.g. AccessType.WRITE). | ||
| :returns: The access mapping to be used by this API. | ||
| :rtype: Dictionary of strings | ||
| def get_access_mapping(self) -> dict[str]: |
There was a problem hiding this comment.
Do we still need this? Shouldn't the code get it directly form constants?
| This is used to provide the user with API specific error messages. | ||
| :returns: The mapping of access types to API-specific strings. | ||
| :rtype: Dictionary of strings | ||
| def get_reverse_access_mapping(self) -> dict[str]: |
| api_config = Config.get().api_conf() | ||
| rev_access_mapping = api_config.get_reverse_access_mapping() | ||
| return rev_access_mapping.get(self, str(self).lower()) | ||
| rmap = Config.get().api_conf().get_reverse_access_mapping() |
There was a problem hiding this comment.
get_constants().REVERSE_ACCESS_MAPPING ?
sergisiso
added
ready for review
reviewed with actions
and removed
under review
ready for review
labels
No description provided.