Skip to content

- Change defaults for some options #574

Open
alxogm wants to merge 1 commit intomainfrom
qq_change_defaults
Open

- Change defaults for some options #574
alxogm wants to merge 1 commit intomainfrom
qq_change_defaults

Conversation

@alxogm
Copy link
Contributor

@alxogm alxogm commented Nov 10, 2022

Changed the defaults for options
--zbest, --bbflux and --desi_footprint to be true as defult. Now in order to set them false we have to give --no-zbest, --no-bbflux, and no-desifootprint.
@andreufont @HiramHerrera @andreicuceu I think those are the only arguments that make sense to change for now... let me know if there are other suggestions.

I also updated a bit the notebook about reproducibility to work with the latest version of the mocks. I simplified the notebook and included some information about the truth files. This can be improved in a future PR is we want to...

  and updated the notebook about reproducibility
@andreufont andreufont self-requested a review November 10, 2022 07:09
@andreufont
Copy link
Contributor

Not sure why the tests are failing, but I agree with the changes in the defaults (although we could also delete the options from argparser).

@alxogm
Copy link
Contributor Author

alxogm commented Nov 10, 2022

It fails because the functionality is only available in python 3.9 and the tests are in 3.8.

@sbailey is there any reason to keep the tests with python 3.8? I see other packages like desispec are already in 3.9...

@andreufont
Copy link
Contributor

@alxogm - is this still a relevant PR? Was it addressed elsewhere?

@sbailey
Copy link
Contributor

sbailey commented Dec 19, 2025

Catching up on ancient PRs — @alxogm @andreufont @abrodze @andreicuceu do we still want this, or is it irrelevant at this point? Go ahead and merge if you want it, or otherwise please close it. Don't worry about github unit tests; we'll clean those up separately since they aren't directly related to these changes (and in some cases are due to python versions we don't support anymore...)

@abrodze
Copy link
Member

abrodze commented Dec 19, 2025

After chatting with @HiramHerrera -- this PR is a harmless update; however, let's not merge it until after unblinding, or the change will need to be propagated through the entire Lya mock production pipeline for DR2.

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