Skip to content

feat: make catalog_path configurable in plot_catalog function for 4FDGL Fermi catalog#60

Merged
tianluyuan merged 3 commits into
mainfrom
feature/optional-catalog-path
Sep 11, 2025
Merged

feat: make catalog_path configurable in plot_catalog function for 4FDGL Fermi catalog#60
tianluyuan merged 3 commits into
mainfrom
feature/optional-catalog-path

Conversation

@azegarelli

@azegarelli azegarelli commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

This PR introduces an optional catalog_path argument in the plot_catalog function.


This change is particularly crucial for Docker containers that automatically process track alerts:

  • Containers can now access and use catalogs directly, without depending on the static CATALOG_PATH.
  • Without this option, it is impossible for containerized scripts to use plot_catalog flexibly (e.g. for plotting 4FGL). Indeed, up to now we’ve had to run this functionality manually in our own Condor spaces. With the proposed change, Docker containers will be able to automatically generate plots with Fermi gamma-ray sources overplotted on the best-fit direction and map.

Benefits

  • Increases flexibility in catalog version usage
  • Simplifies integration with automated pipelines and containerized workflows
  • Maintains backward compatibility (default path still works)

@azegarelli azegarelli requested review from G-Sommani, dsschult, ric-evans and tianluyuan and removed request for tianluyuan September 9, 2025 12:29
@azegarelli azegarelli changed the title feat: make catalog_path configurable in plot_catalog function feat: make catalog_path configurable in plot_catalog function for 4FDGL Fermi catalog Sep 9, 2025

@G-Sommani G-Sommani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ric-evans, do you know why so many checks are failing? For sure it is not because of this modification.

@azegarelli

Copy link
Copy Markdown
Contributor Author

@ric-evans, do you know why so many checks are failing? For sure it is not because of this modification.

thanks @G-Sommani for pointing this out.
indeed, these failures seem to be related to the latest Dependabot pull requests.

@ric-evans

Copy link
Copy Markdown
Contributor

back from vacation -- I'll fix these failures in another branch

@tianluyuan tianluyuan merged commit 9fe5e73 into main Sep 11, 2025
19 checks passed
@tianluyuan tianluyuan deleted the feature/optional-catalog-path branch September 11, 2025 18:25
@azegarelli

Copy link
Copy Markdown
Contributor Author

@tianluyuan I realized I need to update also https://github.com/icecube/skyreader/blob/main/skyreader/plot/plot.py to allow the user to properly pass the catalogh path variable.
I'm restoring the branch

@azegarelli azegarelli restored the feature/optional-catalog-path branch September 11, 2025 19:10
@azegarelli

Copy link
Copy Markdown
Contributor Author

Now the branch contains all the changes I need.
Please don’t remove the branch, even after it’s merged. Whether I realize that something else needs to be modified (though I don’t think so), I can continue making changes in the same branch. Thanks :)

@ric-evans

ric-evans commented Sep 11, 2025

Copy link
Copy Markdown
Contributor

Now the branch contains all the changes I need. Please don’t remove the branch, even after it’s merged. Whether I realize that something else needs to be modified (though I don’t think so), I can continue making changes in the same branch. Thanks :)

The branch is auto-deleted when the PR merges. You would need to create a new branch from main, then make a new PR; ie: feature/optional-catalog-path-2

@azegarelli

Copy link
Copy Markdown
Contributor Author

Now the branch contains all the changes I need. Please don’t remove the branch, even after it’s merged. Whether I realize that something else needs to be modified (though I don’t think so), I can continue making changes in the same branch. Thanks :)

The branch is auto-deleted when the PR merges. You would need to create a new branch from main, then make a new PR; ie: feature/optional-catalog-path-2

ok thanks :) I didn't know

azegarelli added a commit that referenced this pull request Sep 12, 2025
Following the previous [pull
request](#60), this change has
been made:
_catalog_path_ is passed directly as an argument to the functions in
_plot.py,_ which are the ones actually used by Skymist and with which it
can interact
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.

4 participants