Conversation
|
This is not specific to this PR. I'm posting this to all open PRs. Please be aware of the update to our AI usage policy, specifically on the disclosure of and acceptable uses of AI. |
|
pre-commit.ci autofix |
samaloney
left a comment
There was a problem hiding this comment.
Thanks for the PR the test fails are unrelated but do tests pass locally for you?
Note I've run the pre-commit.ci autofix so you will need to pull these change back down. Needs a changelog too.
I think the level handling of the SOLO data could be improved there must be level keyword/variable that can be checked.
|
Most tests passed, there are some fails related to upstream sunpy The new commit includes some level handling and a changelog. |
|
pre-commit.ci autofix |
6c00e49 to
edac3d7
Compare
|
Ok I've update this PR with the merge Fido fixes and the tests look good. I think the only thing this needs is a test similar to what's done here https://github.com/sunpy/radiospectra/blob/main/radiospectra/spectrogram/sources/tests/test_psp_rfs.py |
| "times": times, | ||
| "freqs": hfr_frequency, | ||
| return data, meta | ||
| elif "L2" in data_type: |
There was a problem hiding this comment.
I definitely made an assumption about the contents of the cdf files there, will double check and revert back as needed
There was a problem hiding this comment.
This is what I get when running the plotting code:
There was a problem hiding this comment.
yeah that doesnt look right, thanks for checking Herman! @samaloney can you remember this working in the past? i suggest we only add support for L3 data (which is what the RPW team suggest scientists to use anyway)
There was a problem hiding this comment.
No it still works, the default colour scheme prob look messy for that specific data if you use the example from the source you get this.
Also if you use LogNorm the above data do show things but we just don't interpolate fill in the missing data.
We should point user towards the L3 file but may not be the best for every one.
Add handling of SOLO RPW surv flux products (TNR-SURV-FLUX & HFR-SURV-FLUX) sunpy#136 Change SOLO L3 handling to use sunpy sfu
for more information, see https://pre-commit.ci
Add leveling handling for L2 and L3 data products Add changelog file for new feature
for more information, see https://pre-commit.ci
Adds unit tests for level 3 SOLO RPW data products
f4b5aef to
5360361
Compare
samaloney
left a comment
There was a problem hiding this comment.
Looks good to me, thanks for the PR.
Now we just need to add a Fido client to allow search and download of the L3 data.
|
we can get it from SOAR @samaloney - dont need specific client |
|
@Hermanlrx thanks for the PR great to get the L3 SO/RPW support 🚀 |
Ah great I didn't think/know it was in the SOAR |
Thanks for the help @samaloney, to many more ;) 🍻 |

-Add handling of SOLO RPW surv flux products (TNR-SURV-FLUX & HFR-SURV-FLUX) #136
These changes allow you to read and plot the new data products using Spectrogram.
Long term it could be good to separate the TNR and HFR data products depending on usage.