XTCE decom with CoDICE example

[GFMoraga]

Change Summary

You will notice a few commits, which were organizing, formatting, and editing code. This is the Python script to generate an XTCE formatted XML file, as well as tests and decom using space-packet-parser.

Overview

Matthew and I went through and formatted the directories to match swe. Organized for simplicity and renamed for clarity.

New Files

• make_xtce.py

  • This is the script that extracts an excel sheet and uses it to build the xtce file

• p_cod_aut_test.xml

  • This is the xtce formatted file which is the output of make_xtce.py

• decom.py

  • This uses the space-packet-parser to parse the data with the xtce

• test_decom.py

  • This is a decom to test the data for ccsds headers

• test_bin_data.py

  • This tests the data bin and will output the bin size in bytes, as well as print the data in hexagon

• decom_python_example.py

  • This is an example of how to hard code parse

Testing

• We tested this in a poetry environment with python 3.11

[bourque]

At first glance, I think this all looks pretty good and is well on its way!

Just a few overarching 'best practices' / good habits suggestions:

1. It is good for every function (with a few exceptions) to have docstrings that describe what the function does, and what parameters/returns it has (if any). These docstrings should go just below the function definition (see example here), instead of before or after the function.
2. Within the function docstrings, make sure they are using the numpydocs convention, for example:

Parameters
----------

instead of

Parameters:

I believe this is actually necessary in order for Sphinx/ReadTheDocs to render properly.

3. Though PEP8 doesn't talk about this, I think it is convention within the python community to order functions within a module in alphabetical order (e.g. decommute_packet() should go before extract_bits())

[bourque]

Nice work here @GFMoraga! Thank you for pioneering this effort for CoDICE!

I have a few suggestions that I think should be immediately resolved (mostly using relative directory paths instead of absolute), but many of them are 'future considerations' that I think could be done in a future PR.

[maxinelasp]

Overall, the logic looks good! I suggested some things to rearrange the way the code is laid out, to help make this code more useful for other instruments.

[maxinelasp]

It is best practice to have the main function be really small and call out to other methods (IMO). Ideally, this would look something like:

df = parse_excel_file(filename)
header = generate_header()
integer_parameter_types = generate_integer_parameter_types()
codice_parameters = generate_codice_parameters()
tree = generate_xml_tree(df, header, integer_parameter_types, codice_parameters)
tree.write(...)

This is just an example, split out the code in methods that make sense to you and produce nice code. By splitting everything up into methods, you make it easy for me to create a make_mag_xtce.py which uses all the common tools you created. It also makes it a lot more clear what you're actually running when you run this file.

[tech3371]

I agree here.

[tech3371]

I want to say, this looks really promising. It seems to what we want with minor changes. Nice work! If you need any help with refactoring, let us know.

[tech3371]

I agree here.

[tech3371]

Is there way to go through each row in lengthInBits column and use that to create IntegerParameterType? That way, if there are int size greater than 32 or unique value after that, they would get created.

[laspsandoval]

Yes. I agree. In general we want this to be more generic (not hard-coded).

[tech3371]

We need to add pandas, openpyxl to poetry dependency list.

[laspsandoval]

Update pyproject.toml to include pandas (maybe others too).

[laspsandoval]

It would be nice to have a list of sheet names here.

[GFMoraga]

@maxinelasp @tech3371 @laspsandoval @bourque I love all of these comments and suggestions. I worked in a lot of them. I did NOT get to some because this is a beginning, not a final version. I added some new files and took away the test_decom because it was not useful.
I plan to reopen a new PR to add more classes, move things around and update code for a broader use of instruments.

[maxinelasp]

This looks good for a first pass! Make sure you comment on your issue or create a new issue with any follow ups that you didn't finish from this code review, so we can track that.

You already reviewed it

[GFMoraga]

pre-commit.ci autofix

[bourque]

I agree with Maxine -- nice work, and let's keep track of what improvements we can make.

A few off the top of my head:

1. Making the tests more in the style of unit tests, and maybe adding more of them if there are more areas we can test
2. Moving docstrings around to be compliant with PEP257/numpydocs
3. Adding said documentation to the imap_processing docs (as well as any other outward-facing docs we want to add)
4. Utilizing classes, config files, or argument parsing to abstract out parameters from the code
5. Make the code more generalized, generally speaking

[greglucas]

Nice work, @GFMoraga 🎉

Just an FYI that we shouldn't merge code unless all of the checks are green (unless you leave an explicit note in the PR saying "test X is failing and that is a known failure due to Y" or something like that). This now causes failures for everyone working on dev, so don't hesitate to reach out if you need help fixing linting problems. A more minor thing is that there are now a bunch of commit messages saying "fixed formatting" which aren't super helpful, so it'd be nice to squash merge if you have a bunch of unnecessary commits https://github.com/IMAP-Science-Operations-Center/imap_processing/commits/dev Let me know if any of this doesn't make sense and I'm happy to help out too!

Great work on your first PR, these are minor issues I'm mentioning that can be addressed in follow-ups.