Skip to content

Conversation

@rknop
Copy link
Collaborator

@rknop rknop commented May 27, 2025

No description provided.

@rknop rknop marked this pull request as ready for review May 27, 2025 19:45
@rknop rknop requested a review from laldoroty May 27, 2025 19:45
CITATION.cff Outdated
cff-version: 1.2.0
message: "If you use this software in your work, please cite it using the following metadata."
authors:
- family-names: "Sosey"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be megan or me/rob/lei/michael/shu/etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should be you et al. Will edit.

@@ -0,0 +1,9 @@
# automatically requests pull request reviews for files matching the given pattern; the last match takes precendence. Make sure the software-admin team has write on your repo

* @laldoroty @Roman-Supernova-Pit/software-admins
Copy link
Collaborator

Choose a reason for hiding this comment

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

is "unknown owner" a problem here? do I need to be added to software-admins?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Honestly, I'm not sure what the story is here... we shouldn't add everybody who is the owner of a package to the software-admins group. I'll have to talk to Megan about this, but for now, let's ignore the bug.

@@ -0,0 +1,3 @@
def test_version_is_string():
Copy link
Collaborator

Choose a reason for hiding this comment

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

never occurred to me to write a test for this...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This comes out of the package template -- it has some built in tests to make sure you did the version bumping correctly.

@@ -0,0 +1,202 @@
[project]
name = "phrosty"
description = "Difference imaging forced photometry pipeline"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we add "PHotometry for ROman with SFFT for tYpe Ia supernovae" in the description or nah?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nah. It's in the README.rst, and we'll put it in the narrative docs when we write them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

are we nuking requirements.txt entirely or are the requirements somewhere else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nuked for now, because they were wrong. (The package template put in some defaults, but they were wrong.) Eventually they should probably go back. But, for our own purposes, we'll be running in the snpit env, so we know that the requirements will be satisfied.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we sets up somewhere else, then?

README.rst Outdated
=============================================

**PHotometry for ROman Simulations protoTYpe.**
phrosty: **PHotometry for ROman Simulations protoTYpe.**
Copy link
Collaborator

Choose a reason for hiding this comment

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

PHotometry for ROman with SFFT for tYpe Ia supernovae

README.rst Outdated

*phrosty* requires commit `74a9053` from `roman_imsim`, which may be git cloned from `https://github.com/matroxel/roman_imsim.git`. (This is nominally `v2.0` of that archive, but as of this writing, there is both a branch and a tag `v2.0`, which confuses the `git archive` command. Thus, we give you the specific commit.)
*phrosty* requires commit ``74a9053`` (*warning*: this may be out of
date) from ``roman_imsim``, which may be git cloned from ``https://github.com/matroxel/roman_imsim.git``. (This is nominally ``v2.0`` of that archive, but as of this writing, there is both a branch and a tag ``v2.0``, which confuses the ``git archive`` command. Thus, we give you the specific commit.)
Copy link
Collaborator

Choose a reason for hiding this comment

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

no longer true---we use roman_imsim main branch now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah! Will fix.

@rknop rknop merged commit 7e0b08b into main May 27, 2025
1 of 2 checks passed
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