romancal 1.0.0

[braingram]

This relates to https://innerspace.stsci.edu/spaces/SCSB/pages/695380542/Versioning+romancal+rad+roman_datamodels

I'm opening an issue to help track work towards the goals discussed in the above document.

[braingram]

@stscijgbot-rstdms

[stscijgbot-rstdms]

This issue is tracked on JIRA as RCAL-1386.

[braingram]

It seems beneficial to start with assessing the modules and what might be either removed, made private or declared public.

Importing an installed romancal imports no submodules which I think is a useful design decision given the size of the package.

The following top-level modules are importable:

Test-related: perhaps we shouldn't package this, I'm not sure if pytest --pyargs romancal works for an installed version, if not we should probably exclude it.

• conftest
• regtest
• tests

Infrastructure related:

• associations: how much of this is used?
• datamodels: all of this looks reasonable to make public
  • filetype
  • fileio
  • library
  • migration
• lib
  • basic_utils: make private? some of this looks to be unused
  • dqflags: already deprecated, remove?
  • engdb: make private?
  • progress: is this used anywhere?
  • save_wcs: make private?
  • signal_slot: used in associations.lib.callback_registry, is this used?
  • suffix: make the add/remove/replace_suffix public
  • wcsinfo_to_wcs: make public
• orientation: make private?
• pipeline: public
• scripts: make module private but keep registering scripts
• skycell: public
• step: public
• stpipe: (perhaps include this with step)
  • core: public
  • integration: public
  • utilities: make private (if used)

individual step-related code

• assign_wcs
• dark_current
• dark_decay
• dq_init
• flatfield
• flux
• linearity
• multiband_catalog
• outlier_detection
• photom
• ramp_fitting
• refpix
• resample
• saturation
• skymatch
• source_catalog
• tweakreg
• wfi18_transient

Most of the step modules follow a pattern of (to use wfi18_transient as an example)

• wfi18_transient_step.py (step code)
• wfi18_transient.py (processing code)

This seems like a helpful pattern to follow and make:

• the step file public
• the processing code public
• make any utility functions private (as a general rule)

Perhaps some places to start are:

• remove deprecated and unused code
• remove unused code with no known or expected downstream usage
• identify steps that don't follow the above pattern and adapt them (or identify exceptions)
• identify what to do about including test code
• determine plans for infrastructure code above

Some of this discussion is likely easier on a wiki or innerspace page where edits can be more easily made and tracked.

@schlafly should I add a section to the above versioning page or start a separate one?

[schlafly]

Thank you Brett!

I'm happy if you want to add to the existing page (or create a new one).

I'll note that I do use the compare_asdf function from the regtest folder occasionally; I think that's the only function from the testing folders that I would consider treating as public.

Most of the association code is not live; the skycell_asn and asn_from_list bits are live but nothing else. @ddavis-stsci , I think it's reasonable to try to cut out most of this code now; do you agree? Is it clear to you what can be removed?

I agree that dqflags should be removed. engdb can be made private (right @stscieisenhamer ?), it's only usable internally anyway. Likewise orientation. signal_slot sounds like it should get axed with the association stuff. I'm not familiar with progress.

I'm happy to continue doing this:

• identify steps that don't follow the above pattern and adapt them (or identify exceptions)

as part of the HLIS process.

[schlafly]

Let's try to do this for romancal v1.

@ddavis-stsci , could you take a look at either removing entirely or removing from the public interface as much of the association code as possible? We need to support the asn_from_list and skycell_asn machinery, but I don't think we ever expect to use most of associations.lib, so we should at least rename that library to start with an underscore to indicate that we don't want external users referencing routines in there. Do you agree? Likewise I think that associations.config, generate, pool, and registry likely should not be public and maybe could be deleted.

@stscieisenhamer , would you be able to do something similar for orientation? I feel like we might just rename orientation to _orientation and remove that whole module from the public API, for example, or maybe there are parts that you would like to preserve as public? But if I understand correctly only SDP can use this and they only use it through the command line interface, so I expect that we don't want to treat the functions in e.g. set_telescope_pointing as public.

@larrybradley , it looks to me from browsing source_catalog and multiband_source_catalog that you thought some about what you thought should be part of the public interface and what should not be part of the public interface. Are you happy with the decisions you made there or should we reduce the public interface there before romancal v1? We'd like to adopt semantic versioning moving forward if possible which encourages us to hide bits of the API that we think may be likely to change in the future.

[larrybradley]

@schlafly I didn't really intend for anything in source_catalog and multiband_catalog to be public except for the Step classes. SourceCatalogStep and MultibandCatalogStep are the only things exposed in __all__. I will go further and prepend all modules in those packages with an underscore to indicate that they are private and should not be imported.

[braingram]

Following up on the --pyargs comment. Installing romancal in a fresh environment results in:

• inconsistent versions of rad (0.31.0) and roman_datamodels (0.30.0)
• --pyargs romancal has 15 failed, 731 passed, 287 skipped, 173 warnings

1 failure is from the inconsistent rad/rdm FAILED resample/tests/test_resample_step.py::test_individual_image_meta - asdf._jsonschema.exceptions.ValidationError: 10.0 is not of type 'integer'
the rest of the failures look like they may be photutils 3 related:

FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog[3-10-True-True] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog[3-10-True-False] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog[7-10-False-True] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog[7-10-False-False] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog_no_detections[True] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog_no_detections[False] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog_invalid_inputs[True] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog_invalid_inputs[False] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog_some_invalid_inputs[True] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_catalog_some_invalid_inputs[False] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_source_injection_catalog[3-10-False-True] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_source_injection_catalog[3-10-False-False] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_source_injection_catalog[2-4-True-True] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property
FAILED multiband_catalog/tests/test_multiband_catalog.py::test_multiband_source_injection_catalog[2-4-True-False] - asdf._jsonschema.exceptions.ValidationError: 'psf_match_reference_filter' is a required property

For the scope of this ticket I think this means we find a way to make these tests private but installed (or document that any "tests" or "conftest" is private).

[schlafly]

@schlafly I didn't really intend for anything in source_catalog and multiband_catalog to be public except for the Step classes. SourceCatalogStep and MultibandCatalogStep are the only things exposed in __all__. I will go further and prepend all modules in those packages with an underscore to indicate that they are private and should not be imported.

Yes, adding underscores to the package names is great. Yes, we're looking at the other modules as well. I think there are a handful of routines in source_catalog.psf that we need to leave as 'public' because we promised that users would be able to use romancal to render a PSF. Those are:
psf.render_stamp
psf.get_gridded_psf_model
psf.add_jitter
If you see other functions that we call from the regression tests during the move to underscored package names we should discuss them as well. Thank you!

[schlafly]

Following up on the --pyargs comment. Installing romancal in a fresh environment results in:

• inconsistent versions of rad (0.31.0) and roman_datamodels (0.30.0)

I think this is probably also the cause of the psf_match_reference_filters failures as well; psf_match_reference_filter was recently added to rad. I don't think it's really photutils 3, which doesn't define the psf_match_reference_filter itself.

For the scope of this ticket I think this means we find a way to make these tests private but installed (or document that any "tests" or "conftest" is private).

Whatever you think best here!

[larrybradley]

The 'psf_match_reference_filter' is a required property errors are coming from MultibandCatalogStep. psf_match_reference_filter is an input to that step used for PSF-matched photometry. Not a photutils 3 issue.

[braingram]

Thanks! Confirmed that this is solely a rad version issue. spacetelescope/roman_datamodels#666 should fix it if we make a patch release.