Skip to content

Conversation

msackerm
Copy link
Contributor

Diffuse spectrum plots can show different extensions of IceCube

@msackerm msackerm requested review from Copilot and jvansanten May 26, 2025 14:33
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the diffuse spectrum plots to support multi-detector extensions by enhancing plot color handling and updating configuration paths.

  • Updated spectrum plotting to use a default color list with cycling colors for different detectors.
  • Modified sensitivity handling to maintain backward compatibility via list wrapping.
  • Changed the Gen2-Radio configuration file path in the factory module and added the corresponding new YAML configuration file.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
toise/figures/diffuse/spectrum.py Updated plotting logic using a default color list and improved type checking for labels.
toise/factory.py Changed the Gen2-Radio config file path to point to the new YAML file.
toise/data/gen2-radio/phase2.yaml Added new configuration data for Gen2-Radio.
Comments suppressed due to low confidence (1)

toise/figures/diffuse/spectrum.py:941

  • [nitpick] The variable name 'cindex' is ambiguous; consider renaming it to 'color_index' to improve clarity.
for cindex,dataset in enumerate(datasets):

poly_label.append(
"{label} ({years:.0f} years)".format(label=label, years=years)
)
if type(label) is str: label=[label]
Copy link

Copilot AI May 26, 2025

Choose a reason for hiding this comment

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

Consider using isinstance(label, str) instead of type(label) is str for improved readability and better support of inheritance.

Suggested change
if type(label) is str: label=[label]
if isinstance(label, str): label=[label]

Copilot uses AI. Check for mistakes.

"Gen2-Phase2-Radio": dict(
geometry="Radio",
config_file=os.path.join(os.path.dirname(__file__), "radio_config_16.yaml"),
config_file="data/gen2-radio/phase2.yaml",
Copy link

Copilot AI May 26, 2025

Choose a reason for hiding this comment

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

Using a hard-coded relative path may reduce portability across operating systems; consider using os.path.join to construct the file path dynamically or confirm that the current project structure supports this path.

Suggested change
config_file="data/gen2-radio/phase2.yaml",
config_file=os.path.join(data_dir, "gen2-radio", "phase2.yaml"),

Copilot uses AI. Check for mistakes.

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.

1 participant