Conversation
stixcore/products/product.py
Outdated
| """Hook called after reading data from FITS. Mixins override and chain via super().""" | ||
| s = super() | ||
| if hasattr(s, "on_deserialize"): | ||
| s.on_deserialize(data) |
There was a problem hiding this comment.
Hum this is a bit of code smell - it's breaking the inheritance pattern - I'd suggestion implement on the base case and either pass or raise not implemented error
There was a problem hiding this comment.
i implemented a hopefully more pythonic flow
Fits de/serialize hook for converting into fits serializeable icrs frame
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #458 +/- ##
==========================================
- Coverage 72.23% 67.28% -4.95%
==========================================
Files 78 80 +2
Lines 8085 9023 +938
==========================================
+ Hits 5840 6071 +231
- Misses 2245 2952 +707 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
samaloney
left a comment
samaloney
left a comment
There was a problem hiding this comment.
I still need to check the code out and run it as hard to fully understand from just reading but looks good, left a few minor comments.
| return table_hdu | ||
|
|
||
|
|
||
| def CreateUtcColumn(table, data, colname, description="UTC Time"): |
There was a problem hiding this comment.
snake case
| def CreateUtcColumn(table, data, colname, description="UTC Time"): | |
| def create_utc_column(table, data, colname, description="UTC Time"): |
or maybe create_UTC_column not sure what the PEP says for this?
| CreateUtcColumn( | ||
| data, | ||
| [Time(d, format="isot", scale="utc") for d in mt["start_UTC"]], | ||
| "start_UTC", | ||
| description="start time of flare", | ||
| ) |
There was a problem hiding this comment.
data.add_column(Column([Time(d, format="isot", scale="utc") for d in mt["start_UTC"]], name="start_UTC",description="start time of flare")
not sure if the function is need id honest really do much?
| return kwargs["level"] == "L3" and service_type == 0 and service_subtype == 0 and ssid == 8 | ||
|
|
||
|
|
||
| def longest_constant_sequence(state_array): |
There was a problem hiding this comment.
test for this would be good
| return max_length, max_start_idx, max_state | ||
|
|
||
|
|
||
| def calculate_overlap(range1, range2): |
There was a problem hiding this comment.
test for this would be good
| # do the calculations with stixpy | ||
| try: | ||
| stixpy_cpd = STIXPYProduct(Path(data[i]["cpd_path"])) | ||
| time_range = TimeRange(max(peak_time - 20 * u.s, start_time), min(peak_time + 20 * u.s, end_time)) |
3 participants
No description provided.