DM-15831: Use astropy for ra/dec string parsing #18
Conversation
| @@ -203,31 +211,34 @@ def std_raw(self, item, dataId): | |||
| return item | |||
|
|
|||
| def std_dark(self, item, dataId): | |||
| """Standardiation of master dark frame. Must only be called on master darks. | |||
| """Standardiation of master dark frame. Must only be called on master | |||
There was a problem hiding this comment.
I know it's not your typo, but while you're touching this could you do Standardiation --> Standardization please?
|
|
||
| @param[in] md image metadata | ||
| @return sanitized and concatenated filter name | ||
| """ | ||
| filt1 = self._translate_filter(md.getScalar("FILTER1").rstrip().lstrip()) | ||
| filt2 = self._translate_filter(md.getScalar("FILTER2").rstrip().lstrip()) | ||
| sorted_filter = [filt1, filt2] | ||
| sorted_filter.sort() #we want to be insensitive to filter order, for now at least |
There was a problem hiding this comment.
I think comments like this are allowed as the line was under length, and it's not a "block comment" but I don't really mind that it's been changed, just wanted to check I hadn't misunderstood the rules...
There was a problem hiding this comment.
You are right that I didn't have to shift the comment to its own line. The reason I did so is personal preference. For longer commentary I prefer explaining the reasoning before the code line happens, rather than having the reason as an addendum.
| @@ -0,0 +1,11 @@ | |||
| [flake8] | |||
There was a problem hiding this comment.
Thanks for setting this up as part of this ticket 🙏
|
Looks good. I didn't see a PR for the |
|
Thanks for the review. DM-15831 is the more general "code removal" ticket and hasn't been approved in RFC yet. This change is needed for that but given that this PR seems like a good thing to do independent of the RFC being approved it seemed worth doing anyhow. Thanks for looking at the change. |
timj
force-pushed
the
tickets/DM-15831
branch
from
45c5eeb
to
200b99f
Compare
Also flake8 clean the code.