Skip to content

ADR 3: Position sampling and patches of the sky #422

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

Merged
merged 5 commits into from
Feb 24, 2021

Conversation

ntessore
Copy link
Member

@ntessore ntessore commented Feb 5, 2021

ADR for the sampling of positions on the sky. Direct link to proposal here.

@ntessore ntessore added the ADR Architecture Decision Record label Feb 5, 2021
@ntessore ntessore requested review from CKrawczyk and rrjbca February 5, 2021 21:43
Copy link
Contributor

@CKrawczyk CKrawczyk left a comment

Choose a reason for hiding this comment

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

I have pointed out some points that can be clarified above. It would also be nice if this ADR added:

  • "Status = pending" section that is updated to "accepted" when the review is done, and "implemented" when it makes it into the code
  • a section for "Consequences" that goes into how these changes would affect the existing code (could be "does not affect existing code").

@rrjbca rrjbca changed the base branch from master to main February 22, 2021 13:38
CKrawczyk
CKrawczyk previously approved these changes Feb 23, 2021
Copy link
Contributor

@CKrawczyk CKrawczyk left a comment

Choose a reason for hiding this comment

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

Looks good to me.

rrjbca
rrjbca previously approved these changes Feb 24, 2021
Copy link
Contributor

@rrjbca rrjbca left a comment

Choose a reason for hiding this comment

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

I like this design. The implementation of e.g. region batches will require some thought but the proposal seems flexible enough to accommodate this. I will update to "Status: Approved" and merge.

@rrjbca rrjbca dismissed stale reviews from CKrawczyk and themself via faf483c February 24, 2021 14:27
@rrjbca rrjbca merged commit f99783e into skypyproject:main Feb 24, 2021
@ntessore ntessore deleted the adr-03 branch February 24, 2021 16:17
itrharrison added a commit that referenced this pull request Dec 15, 2022
* Update name of default branch to main (#434)

* update mailmap (#432)

* Write all tables to a single FITS/HDF5 file (#425)

* ADR 3: Position sampling and patches of the sky (#422)

* BUG: Raise ImportError if optional dependency speclite is not installed (#437)

* MAINT: Set NumPy latest supported version to 1.20 #440

* Update status badges (#441)

* MAINT: Update Lucia affiliation (#451)

* MAINT: add SIT's information (#450)

* DOC: Fix contributor guidelines link (#449)

Co-authored-by: Richard R <[email protected]>
Co-authored-by: Nicolas Tessore <[email protected]>

* ENH: Logging for skypy command line script and Pipeline class (#453)

* DOC: Describe speclite filters in documentation (#457)

* ENH: Config syntax for importing objects (#463)

* DOC: List of Features (#456)

* DOC: How to construct config files (#454)

* DOC: Remove docstring examples (#429)

* MAINT: Update Zenodo entry for RPR (#468)

* DOC: Readme updates (#460)

* DOC: Expanded landing page documentation (#228)

* DOC: Inverse transform sampling accuracy warning (#472)

* MAINT: Set astropy latest supported version to 4.2 (#483)

* DOC: zenodo json members update (#481)

* DOC: Ryden04 ellipticity doc missing section (#477)

* MAINT: Update numpy and scipy latest supported versions (#488)

* BUG: Change invalid ecsv datatype from int to uint16 (#485)

* DEV: setuptools==58.0.0 (#493)

Co-authored-by: Nicolas Tessore <[email protected]>

* Add compatibility workflow badge (#487)

* DEV: Enable pip to install pre-releases in the tox dev environments (#491)

* TST: Use tmp_path fixture for temporary files in unit tests (#489)

* BUG: Move handling of context arguments after handling of .depends keyword (#465)

* BLD: Set astropy latest supported version to 4.3 and speclite minversion to 0.14 (#486)

* REV: restore setuptools to latest version on readthedocs (#494)

* DEV: pyparsing<3.0.0 (#500)

* Check new astropy file overwrite error message in logging test (#498)

* REV: restore pyparsing to latest version for doc builds (#501)

* DOC: Update citation file with JOSS paper reference (#496)

* BLD: Set astropy latest supported version to 5.0 (#504)

* BLD: Set python latest supported version to 3.10 (#505)

* BLD: Set numpy latest supported version to 1.22 (#506)

* BLD: Set python oldest supported version to 3.7 (#507)

* DOC: Fix code of conduct link (#508)

* Changed y-label in luminosity function example. (#512)

* BLD: Set scipy latest supported version to 1.8 (#510)

* ENH: Rykoff model of the magnitude uncertainty (#526)

* TST: assert photometric error is numerically close to the analytic value (#545)

* TST: Drop deprecated astropy.tests.helper.raises (#546)

* ENH: compute kcorrect remaining stellar mass (#476)

* compute kcorrect remaining stellar mass

* added test for stellar mass remain

Co-authored-by: Ian Harrison <[email protected]>
Co-authored-by: Lucia F. de la Bella <[email protected]>

* ENH: Logistic completeness function (#521)

* BLD: Set astropy latest supported version to 5.1 (#547)

* BUG: `schechter_smf` callable input and docs (#525)

* DOC: Typo in Rykoff error (#550)

* add Fox's details (#551)

Co-authored-by: Richard R <[email protected]>

* BLD: Set numpy latest supported version to 1.23 (#552)

* codestyle fixes

* add six requirement for colossus

* tried to fix docs builds

* update passenv

* rtd configuration

Co-authored-by: Richard R <[email protected]>
Co-authored-by: Nicolas Tessore <[email protected]>
Co-authored-by: Lucia F. de la Bella <[email protected]>
Co-authored-by: Sut-Ieng Tam <[email protected]>
Co-authored-by: philipp128 <[email protected]>
Co-authored-by: Fox Davidson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ADR Architecture Decision Record
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants