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

Use galcheat in BTK #228

Merged
merged 77 commits into from
Mar 24, 2022
Merged

Use galcheat in BTK #228

merged 77 commits into from
Mar 24, 2022

Conversation

ismael-mendoza
Copy link
Collaborator

@ismael-mendoza ismael-mendoza commented Oct 6, 2021

Closes #222
Closes #269
Closes #268

  • run poetry update
  • add galcheat as a dependency in pyproject.toml
  • get rid of most survey.py functions that load surveys, etc.
  • get rid of repeated YAML files
  • can use the hydra list config to specify list of surveys that want to be loaded from galcheat.survey_info dict
  • make a new config directory + corresponding YAML files only for the PSF. PSF can be added to survey object/filters after creation

@ismael-mendoza ismael-mendoza changed the title add galcheat to pyproject toml dependencies Use galcheat in BTK Oct 6, 2021
@aboucaud
Copy link
Collaborator

FYI the galcheat API is currently being updated.
Don't rush too quickly into implementing this. :)

@ismael-mendoza
Copy link
Collaborator Author

Thanks @aboucaud - please let us know when the API is more or less stable and I can continue this PR

@codecov
Copy link

codecov bot commented Nov 14, 2021

Codecov Report

Merging #228 (1d18bef) into develop (cf3db28) will increase coverage by 0.15%.
The diff coverage is 94.20%.

@@             Coverage Diff             @@
##           develop     #228      +/-   ##
===========================================
+ Coverage    88.37%   88.53%   +0.15%     
===========================================
  Files           13       13              
  Lines         1540     1526      -14     
===========================================
- Hits          1361     1351      -10     
+ Misses         179      175       -4     
Flag Coverage Δ
unittests 88.53% <94.20%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
btk/survey.py 96.20% <92.30%> (+2.93%) ⬆️
btk/draw_blends.py 93.41% <93.54%> (+0.13%) ⬆️
btk/main.py 95.65% <100.00%> (+0.41%) ⬆️
btk/metrics.py 94.69% <100.00%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf3db28...1d18bef. Read the comment docs.

@ismael-mendoza
Copy link
Collaborator Author

I'm back to working on this issue @aboucaud - I will let you know if I have any questions or if a review might be helpful.

@ismael-mendoza ismael-mendoza added feature add new feature dependencies Pull requests that update a dependency file refactor code refactoring labels Nov 15, 2021
@ismael-mendoza ismael-mendoza added this to the v1.0.0 milestone Nov 15, 2021
@ismael-mendoza
Copy link
Collaborator Author

Hi @aboucaud and @thuiop this is now ready for review. I decided to keep the central wavelenghts as a temporary dictionary for now in btk/survey.py. Once we figure out how they are calculated and added to galcheat we can grab them from there (it should be a pretty quick switch after that). Same with the flux calculations, I'm still using the default ones in BTK until this PR is merged.

All survey information is now grabbed from a survey in galcheat. The psf can still be customized via a psf_func in get_surveys. Please let me know if you have any suggestions!

ismael-mendoza and others added 11 commits March 21, 2022 15:10
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
ismael-mendoza and others added 4 commits March 21, 2022 15:16
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
Co-authored-by: Alexandre Boucaud <[email protected]>
@thuiop
Copy link
Collaborator

thuiop commented Mar 22, 2022

I think this is ready for review @aboucaud and @thuiop I would appreciate any additional suggestions if you have any (or otherwise feel free to approve). All tests are now passing smiley

@thuiop could you help me out with changing the corresponding documentation and notebooks if you have time? (#270) Once this is merged? Thanks!

@aboucaud - do you intend to merge the FList PR from galcheat so that it now works on BTK or should we try to think of an alternative? I know removing it is not the ideal solution for galcheat...

Sure, I can take care of the doc after this is merged. I will also try to make a review tomorrow to make sure we do not miss anything.

@ismael-mendoza
Copy link
Collaborator Author

ismael-mendoza commented Mar 23, 2022

Sure, I can take care of the doc after this is merged. I will also try to make a review tomorrow to make sure we do not miss anything.

Thank you 🙏 I'll merge sometime this morning probably

EDIT: Sorry actually after your review @thuiop

Copy link
Collaborator

@thuiop thuiop left a comment

Choose a reason for hiding this comment

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

I did not have much to comment on actually, it looks mostly good to me.

btk/draw_blends.py Show resolved Hide resolved
@ismael-mendoza
Copy link
Collaborator Author

OK I think this is ready to merge - there are some rough edges with the docs and noteboks described in #270 that we should look into next.

@ismael-mendoza ismael-mendoza merged commit 53c2f76 into develop Mar 24, 2022
@ismael-mendoza ismael-mendoza deleted the use-galcheat branch March 24, 2022 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file feature add new feature refactor code refactoring
Projects
None yet
3 participants