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

Add centroid_dashboard sub-package #8

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

taldcroft
Copy link
Member

Description

This is a minimal port of mica.centroid_dashboard into the ska_trend package. Drivers:

  • This functionality is really trending and there are no user APIs.
  • ska_trend is more flexible for releases and updates.

The purpose of this PR is just to get a baseline copy of the existing code and then start a new PR modifying it.

Interface impacts

Adds a new subpackage centroid_dashboard.

Testing

Unit tests

  • No unit tests

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

From within a dev 2025.0rc2 environment:

✗ pip install .
Processing /Volumes/git/ska_trend
  Preparing metadata (setup.py) ... done
Building wheels for collected packages: ska_trend
  Building wheel for ska_trend (setup.py) ... done
  Created wheel for ska_trend: filename=ska_trend-0.2.1.dev1+g3540380.d20250211-py3-none-any.whl size=24606 sha256=a6fee5f877d954e9b27b9d4e795d59db17b5bbcd310d867ccec038025b84c31d
  Stored in directory: /private/var/folders/0p/mj5zm2311gl283lksrknl7bc0000gp/T/pip-ephem-wheel-cache-jus20bdj/wheels/86/61/7a/c12cc04cf60f2600625c55fc0d075e8bfbd8a5ab44a28103e3
Successfully built ska_trend
Installing collected packages: ska_trend
Successfully installed ska_trend-0.2.1.dev1+g3540380.d20250211

$ ska_trend_centroid_dashboard --help
usage: ska_trend_centroid_dashboard [-h] [--obsid OBSID] [--start START] [--stop STOP] [--data-root DATA_ROOT] [--make_plots | --no-make_plots]
                                    [--save | --no-save] [--force | --no-force]

Centroid dashboard

options:
  -h, --help            show this help message and exit
  --obsid OBSID         Processing obsid (default=None)
  --start START         Processing start date (default=NOW - 7 days)
  --stop STOP           Processing stop date (default=NOW)
  --data-root DATA_ROOT
                        Root directory for data files (default='/proj/sot/ska/www/ASPECT_ICXC/centroid_reports')
  --make_plots          make plots if the option is included
  --no-make_plots       Don't make plots if the option is included
  --save                save plots if the option is included
  --no-save             Don't save plots if the option is included
  --force               force processing if the option is included
  --no-force            Don't force processing if the option is included

$ python -c "import ska_trend.centroid_dashboard.centroid_dashboard"
$ # success 

@jeanconn
Copy link
Contributor

I don't really know how to review this. If the point of it is just to have a baseline, I think it should just stay in its own branch until there is a testable new thing.

@taldcroft
Copy link
Member Author

Just keeping this in a branch is one possibility. I had imagined review on these criteria:

  • Do we want to move centroid_dashboard from mica into this package.
  • Is this port basically functional so that it could become the provider of the centroid_dashboard updates with just a new task_schedule?

I think the answer to both is yes. But if you'd rather just have this PR as the dev branch for the next one, then that's OK. I just never like having things lingering.

@jeanconn
Copy link
Contributor

I think it is fine to move it in here. First it was standalone, then it was in mica, and then I think I also started a branch to put it over here so it makes sense to me to put it here.

For the second question, I think it would also be fine to finish this PR so it is functional with a task schedule and takes over updating the centroid_dashboard data and plots. If that were here we could test it, test install it, and have a working release of ska_trend with this as a functional sub-package. At that point it would make sense to me to have incremental PRs against ska_trend main to improve centroid_dashboard.

But if this isn't functional and testable I think it should stay in a branch until that point.

@taldcroft
Copy link
Member Author

I think there is no value in doing all the work and testing for using this branch to do the flight updates. I'll have to do that all over again for the modernized version, so once is enough.

That's fine, and I'm already developing in a new branch off of add-centroid-dashboard.

@jeanconn jeanconn removed their request for review February 12, 2025 15:06
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.

2 participants