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 coordinates logging to use built in str formatting in logger #5092

Open
Cadair opened this issue Mar 15, 2021 · 1 comment
Open
Labels
coordinates Affects the coordinates submodule Effort Medium Requires a moderate time investment Package Novice Requires little knowledge of the internal structure of SunPy Priority Low Slow action required Refactoring Code changes without affecting API (generally)

Comments

@Cadair
Copy link
Member

Cadair commented Mar 15, 2021

Doing this should remove the need for if statements guarding the logging calls

@ayshih ayshih added coordinates Affects the coordinates submodule Effort Medium Requires a moderate time investment Package Novice Requires little knowledge of the internal structure of SunPy Priority Low Slow action required Refactoring Code changes without affecting API (generally) labels Mar 15, 2021
@ayshih
Copy link
Member

ayshih commented Mar 15, 2021

This is code in question:

# Check if the logging level is at least DEBUG (for performance reasons)
debug_output = log.getEffectiveLevel() <= logging.DEBUG
if debug_output:
# Indention for transformation layer
indention = u"\u2502 " * _layer_level
# For the input arguments, add indention to any lines after the first line
from_str = str(args[0]).replace("\n", f"\n {indention}\u2502 ")
to_str = str(args[1]).replace("\n", f"\n {indention}\u2502 ")
# Log the description and the input arguments
log.debug(f"{indention}{description}")
log.debug(f"{indention}\u251c\u2500From: {from_str}")
log.debug(f"{indention}\u251c\u2500To : {to_str}")
# Increment the layer level to increase the indention for nested transformations
_layer_level += 1
result = func(*args, **kwargs)
if debug_output:
# Decrement the layer level
_layer_level -= 1
# For the output, add intention to any lines after the first line
out_str = str(result).replace("\n", f"\n {indention} ")
# Log the output
log.debug(f"{indention}\u2514\u2500Out : {out_str}")

In principle, the log.debug() calls don't need the if debug_output: conditions, but we need to call log.debug() correctly such that it doesn't do any premature string formatting that will trigger __repr__() calls for the frame instances. This may be particularly tricky because I actually modify the __repr__() output:

# For the input arguments, add indention to any lines after the first line
from_str = str(args[0]).replace("\n", f"\n {indention}\u2502 ")
to_str = str(args[1]).replace("\n", f"\n {indention}\u2502 ")

We should also benchmark before/after the refactor to make sure that the change isn't detrimental.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coordinates Affects the coordinates submodule Effort Medium Requires a moderate time investment Package Novice Requires little knowledge of the internal structure of SunPy Priority Low Slow action required Refactoring Code changes without affecting API (generally)
Projects
None yet
Development

No branches or pull requests

2 participants