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

Remove all references to central store #83

Open
wants to merge 1 commit into
base: dr5b
Choose a base branch
from
Open

Conversation

jotaylor
Copy link
Collaborator

Some scripts were wholesale removed (and instead stored in the private ullyses_dp repo). Other references to central store were turned into run-time arguments.

@jotaylor jotaylor requested a review from tapastro August 24, 2023 15:34
@jotaylor
Copy link
Collaborator Author

@wfischer-stsci can you confirm the change I made to lcogt_uphot.pro were done correctly? Sincerely, a python user.

@tapastro
Copy link
Collaborator

There are also some data csv files containing refs - are those no longer used once the changes in this PR are merged?

@wfischer-stsci
Copy link
Collaborator

OK, I'm grasping this a bit better now. The problem is that the function get_counts needs to know about datadir. As written, the user supplies datadir when they call the main program lcogt_uphot but that information does not get passed along to get_counts.

We could make that happen.

But can we just use the lcogt_uphot.pro that's in the lcogt branch? That has been tested to give the same results as previous versions, but the code is better organized.

@wfischer-stsci
Copy link
Collaborator

Tyler has a good point -- a file like V-BP-TAU_cal_fields.csv has a lot of references to central store in it.

Each line has an MJD, an image of a monitoring star obtained on that MJD, and the specific external calibration field that's supposed to go with it. One could remove every recurrence of /astro/ullyses/lcogt_data/ from that file and have the code supply that part of the filename instead.

@jotaylor
Copy link
Collaborator Author

jotaylor commented Nov 20, 2023

@tapastro were there any other requested changes for this branch, or can it be merged? I believe Will removed the central store references in the lcogt branch which was just merged into main :)

@tapastro
Copy link
Collaborator

This PR is against the dr5b branch - I haven't kept up with the state of this release, so I'd have to take a look. Are these changes being applied to main as well, or are they already there or not applicable?

@jotaylor
Copy link
Collaborator Author

This PR is against the dr5b branch - I haven't kept up with the state of this release, so I'd have to take a look. Are these changes being applied to main as well, or are they already there or not applicable?

These change have not been applied to main, but maybe they should be (minus the changes that are now obsolete). Removing references to central store is a good thing to do no matter what. I can chat with you sometime about the open branches in this PR, and I'll just leave this for now.

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.

3 participants