Add tool for migrating stellarphot v1 data to stellarphot v2 #471
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #471 +/- ##
==========================================
+ Coverage 80.05% 80.32% +0.27%
==========================================
Files 31 32 +1
Lines 4011 4067 +56
==========================================
+ Hits 3211 3267 +56
Misses 800 800 ☔ View full report in Codecov by Sentry. |
| @@ -438,6 +438,9 @@ def __init__( | |||
| **kwargs, | |||
| ) | |||
|
|
|||
| # From this point forwarsd we should be using self to get at any data | |||
There was a problem hiding this comment.
| # From this point forwarsd we should be using self to get at any data | |
| # From this point forward we should be using self to get at any data |
Same typo noted in #470
stellarphot/conftest.py
Outdated
There was a problem hiding this comment.
A little confused why you created a fixture for this rather than just creating a new and old version of the testing file... it it just to allow iteration of test cases via the fixture?
There was a problem hiding this comment.
Looks like a through test suite as far as I can tell, kind of explains the weird fixture I noted earlier.
There was a problem hiding this comment.
Yeah, was using it a few different times
| # Note that this deliberately discards any unit that might have been | ||
| # present in the original data. This is a stellarphot-specific migration | ||
| # tool, not something more general, and the choices above are the correct | ||
| # units for the data. | ||
| new_data[un] = new_data[un].value * unitify[un] |
There was a problem hiding this comment.
Could we potentially have unit conversion issues here? Or were these units also used in v1 but just not attached to the columns? I don't remember.
There was a problem hiding this comment.
The old almost never included units, but it depended on whether the photometry was saved as CSV (no units saved) or FITS (units sometimes present for some things). This is maybe a little risky, but for feder data the image unit was always ADU, times, in seconds, etc.
No one else ever used v1 as far as I know, so this should be ok, I think
mwcraig
dismissed
JuanCab’s stale review
The merge-base changed after approval.
Co-authored-by: Juan Cabanela <[email protected]>
Co-authored-by: Juan Cabanela <[email protected]>
This adds a way to update v1 data so that it can be used in v2. This will help remove the need to redo a bunch of photometry while allowing users to take advantage of the new stuff in v2.
NOTE: #470 should be merged first, because that fix was necessary for this PR to work.