deprecate preserve_comments fix spec parsing for inline comments with closing parenthesis

[braingram]

Fixes #105

The preserve_comments argument ends up mapping to the _inspec argument for ConfigObj. Setting this appears to break parsing of spec lines that contain inline comments with a closing parenthesis. The preserve_comments argument also appears to have no effect on parsing comments (aside from introducing the bug above).

This PR deprecates preserve_comments and removes internal usage of the _inspec argument. This should fix parsing inline comments for closing parenthesis (this is tested in the included test) and have no effect on comment parsing (also included in the test).

Doing a quick search through romancal and jwst I see no usage of the preserve_comments argument so I don't expect either of those packages to require updates. Hopefully regression tests (to be run below) will reveal if there are any issues with these changes.

[codecov]

Codecov Report

Patch coverage is 93.75% of modified lines.

Files Changed	Coverage
src/stpipe/step.py	66.66%
src/stpipe/cmdline.py	100.00%
src/stpipe/config_parser.py	100.00%
src/stpipe/pipeline.py	100.00%
src/stpipe/utilities.py	100.00%

📢 Thoughts on this report? Let us know!.

[braingram]

docs failure should be fixed with: #109

[braingram]

Adding @hbushouse for a JWST review
JWST regression tests: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/904/
JWST tests passed with no errors

Adding @WilliamJamieson for a Roman review
Roman regression tests: https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/339/
Roman tests show 1 unrelated error (the one that pops up randomly when the tests are run with xdist):

FAILED romancal/regtest/test_linearity.py::test_linearity_step - ValueError: Block '1' not found.

[hbushouse]

Looks OK to me. I'm just wondering if removing preserve_comments will have any effect on what you get when just listing the params for a given step or pipeline. For example, right now executing strun -h jump gives you the big list of outputs:

  --min_sat_area        minimum required area for the central saturation of snowballs
  --min_jump_area       minimum area to trigger large events processing
  --expand_factor       The expansion factor for the enclosing circles or ellipses
  --use_ellipses        deprecated
  --sat_required_snowball 
                        Require the center of snowballs to be saturated
  --min_sat_radius_extend 
                        The min radius of the sat core to trigger the extension of the core
  --sat_expand          Number of pixels to add to the radius of the saturated core of snowballs
  --edge_size           Size of region on the edges of NIR detectors where a sat core is not required
  --find_showers        Turn on shower flagging for MIRI
  --extend_snr_threshold 
                        The SNR minimum for detection of extended showers in MIRI
  --extend_min_area     Min area of emission after convolution for the detection of showers
  --extend_inner_radius 
                        Inner radius of the ring_2D_kernel used for convolution
  --extend_outer_radius 
                        Outer radius of the ring_2D_Kernel used for convolution

where the comments on each param are clearly displayed. Will those go away with this change?

[braingram]

strun -h jump

Thanks for taking a look and the questions.

The argument parsing and help output appears unchanged with this PR. Running strun -h jump using stpipe installed from this PR I see output that is identical to the output when run with stpipe main branch.

The spec is parsed using ConfigObj which stores inline comments in the inline_comments attribute. That attribute is used when constructing the argument parser (and help output) here:

stpipe/src/stpipe/cmdline.py

Line 90 in 1ade408

comment = subspec.inline_comments.get(key) or ""

[braingram]

Thanks @WilliamJamieson and @hbushouse for the reviews.