SPHEREx PSF notebook update using ePSf - #364
Conversation
troyraen
left a comment
troyraen
left a comment
There was a problem hiding this comment.
I skimmed through it and left a few comments below.
|
|
||
| psf_cube_plane = tab[0]["zone_id"] | ||
| distance_min = tab[0]["distance"] | ||
| # Not sure why this was necessary |
There was a problem hiding this comment.
We should try to understand this and put a more informative comment here. I can help with that if needed.
There was a problem hiding this comment.
I think this is just to convert them to floats.
According to chatGPT, it suggests to just remove the comment and replace it with something like
# Convert to Python floats (accepts ints and NumPy scalar types).
Alternatively, we could do the test:
try:
x0 = float(x0)
y0 = float(y0)
except (TypeError, ValueError) as err:
raise TypeError("x0 and y0 must be real numbers.") from err
What do you think? For now I just did option 1 (remove comment and replace it)
| ```{code-cell} ipython3 | ||
|
|
||
| ``` |
There was a problem hiding this comment.
| ```{code-cell} ipython3 | |
| ``` |
There was a problem hiding this comment.
not sure what's the difference?
troyraen
added
maintenance
jkrick
left a comment
jkrick
left a comment
There was a problem hiding this comment.
small changes requested, otherwise looks good.
|
All comments above taken into account. @troyraen please have another look at the Exception comment you posted above to see if that makes sense. To do:
|
3 participants
update to use of new ePSF.
Currently, data is not public, so the notebook is run on local files.
Leaving this as a draft PR until the data is public so it's not accidentally merged before then.