EEA Instrument code to produce L1A file. #26
Conversation
a cdf file with the raw data
…ermesData container
starting to work with HermesData in conda env add_eea
this write separate records with their code and single records with mine
outputs 1d sep-rec with u.Quantity outputs my data all-in-one-rec
…_1d_uQ, spectra=spectra_EnergyLabels, meta=bare_attrs) It worked for energy labels
2d (EnergyLabels) 3d (ACCUM)
correcting Liams time function a couple of comments
It seems like one was created when I ran the devcontainer on hermes_eea
adding cdflib
|
The processed files are available as an artifact: https://github.com/HERMES-SOC/hermes_eea/actions/runs/7631405077/artifacts/1189877561 |
|
The processed files are available as an artifact: https://github.com/HERMES-SOC/hermes_eea/actions/runs/7631731482/artifacts/1189960659 |
|
The processed files are available as an artifact: https://github.com/HERMES-SOC/hermes_eea/actions/runs/7631753358/artifacts/1189965824 |
|
The processed files are available as an artifact: https://github.com/HERMES-SOC/hermes_eea/actions/runs/7631782826/artifacts/1189972576 |
|
go_plotly is a graphic diff for real warm fuzzies |
|
The processed files are available as an artifact: https://github.com/HERMES-SOC/hermes_eea/actions/runs/7631887739/artifacts/1189994569 |
rstrub
requested review from
dbarrous
and removed request for
ehsteve and
dbarrous
There was a problem hiding this comment.
This PR looks good, reviewed by @Alrobbertz and I. We will approve it once the docstrings are reformatted or added to the newly added functions and classes. Also would greatly appreciate changing the title to something a bit more informative, and adding a bit more to the description of the Pull Request with what has been added and maybe your thoughts and tips of the process so we can use this Pull Request as an example for other instrument teams. Thanks so much again @rstrub! :)
hermes_eea/__init__.py
Outdated
|
|
||
| sys.path.append(os.getcwd()) |
There was a problem hiding this comment.
What was breaking that this needed to be put before importing hermes_core and hermes_eea? Maybe adding a comment would be helpful as well.
There was a problem hiding this comment.
Also if this is not needed, is it possible to remove this?
|
|
||
|
|
||
| # This may eventually be handled in a python multiprocessor module instance: | ||
| def SkymapFactory(l0_cdf, energies, deflections, myEEA): |
There was a problem hiding this comment.
Could you add docstrings to all functions and classes in the format we use so that our automatic documentation can pick it up? An example would be how we use it in this hermes_core function:
https://github.com/HERMES-SOC/hermes_core/blob/3f2c751b6ff1de529adf43a7a216c97be99170a0/hermes_core/util/util.py#L28
| def do_eea_packet( | ||
| stepperTableCounter, counts, cnt1, cnt2, epoch, energies, deflections, ith_FSmap | ||
| ): | ||
| """ |
There was a problem hiding this comment.
Awesome docstrings, but could it be refactored to match the format as mentioned above.
| @@ -74,9 +72,6 @@ testpaths = [ | |||
| "hermes_eea/tests", | |||
| "docs" | |||
| ] | |||
| doctest_plus = "enabled" | |||
There was a problem hiding this comment.
Is it possible to add lines 77-79 back to this file?
| @@ -1,3 +1,4 @@ | |||
| [flake8] | |||
| extend-ignore = E203,E402 | |||
There was a problem hiding this comment.
Just for reference, no action required: psf/black#315
|
The processed files are available as an artifact: https://github.com/HERMES-SOC/hermes_eea/actions/runs/7754769154/artifacts/1214878299 |
|
The processed files are available as an artifact: https://github.com/HERMES-SOC/hermes_eea/actions/runs/7754857267/artifacts/1214897944 |
|
Corrected the linting issues and resolved the failed Windows tests. I adjusted the After these fixes this PR looks good to me and has my approval, any thoughts @ehsteve or @Alrobbertz before squashing and merging? |
EEA Instrument Merge Request. The process of putting CCSDS packets into L1A CDF was already quite familiar to me but the github, python, vscode stuff made it much more uncertain...I wouldn't say difficult... At first it was uncertain because of the whole VScode devcontainer business. I didn't know at first I could easily ignore that stuff and use my own Pycharm IDE. Also, the function names provided in test_calibration.py and calibration.py and some python syntax that was new to me. Near the end it was filled with uncertainty because of much of the github workflow procedures were quite new to me. It is still difficult because I am not even sure if this is the PR description I am supposed to update or not and docstrings are also new to me. The CDFLIB issue was a big sticking point...Over the years I have moved from C->Perl->C++->Python->Javascript->Java->IDL->Python. I think many of my colleagues perhaps have had similar travels. So all these python-centric concepts were troublesome. But I certainly appreciate the role they all play.