Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse comma-separated list as python list #39

Merged
merged 5 commits into from
Jul 25, 2024

Conversation

WalterKolczynski-NOAA
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA commented Jul 25, 2024

Description
Adds parsing of a comma-separated list to a python list to Configuration.

In support of NOAA-EMC/global-workflow#2274

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • pynorms
  • pytests

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes need updates to the documentation. I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

Adds the capability to parse a bash array in a configuration as a
python array.

Note: since arrays cannot reliably be exported in shell, such variables
should only be used locally in a config or by python that is parsing it.
Previous method of reading bash arrays directly did not work because
arrays cannot be exported and the configuration loader works by reading
exported variables. Instead, we now assume any variable with a comma is
a comma-separated list.
Checking for commas had to be moved to first in cast_as_dtype() to
avoid commas being considered part of the datatime string.

Also added pytest for list parsing.
Copy link

codecov bot commented Jul 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.12%. Comparing base (86bfe2b) to head (c68d7dc).
Report is 3 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #39      +/-   ##
===========================================
+ Coverage    50.06%   50.12%   +0.06%     
===========================================
  Files           18       18              
  Lines         1650     1652       +2     
  Branches       337      339       +2     
===========================================
+ Hits           826      828       +2     
  Misses         765      765              
  Partials        59       59              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/wxflow/configuration.py Show resolved Hide resolved
tests/test_configuration.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

I am wondering if this will have an unintended side-effect on g-w where such a string exists in one of the configs.
It should be rectified in the g-w if it does.

@aerorahul aerorahul merged commit 264e8f3 into NOAA-EMC:develop Jul 25, 2024
9 checks passed
@WalterKolczynski-NOAA
Copy link
Contributor Author

looks good.

I am wondering if this will have an unintended side-effect on g-w where such a string exists in one of the configs. It should be rectified in the g-w if it does.

I thought about that, because some of the namelist variables do use comma-separated lists the same way. But we don't really use any of those in the python system (and if we were to use them in python somewhere down the line, we would probably want them in list for too).

WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Jul 25, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Jul 25, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Jul 26, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Jul 30, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Jul 30, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Aug 2, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Aug 5, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Aug 6, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Aug 7, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
WalterKolczynski-NOAA added a commit to WalterKolczynski-NOAA/global-workflow that referenced this pull request Aug 8, 2024
Adds the ability to run a forecast in segments instead of all at
once. To accomplish this, the `FHMIN_GFS` and `FHMAX_GFS` settings
have been replaced as user-setable variables in favor of
`FCST_SEGMENTS_STR_GFS`, a comma-separated list of the segment
boundaries (thus there will be one more than the number of segments).
For a traditional single-segment forecast, this would just be set to
`"${FHMIN_GFS},${FHMAX_GFS}"`.

The comma-separated list had to be used instead of a bash array as
the variable must be exported in order for the rocoto generator to
see it, and arrays cannot be exported from shell. Capabilty to parse
these into python lists was added to wxflow in an accompanying PR.

To accomodate the new segment metatasks that must be run serially,
the capability of `create_task()` was expanded to allow a dictionary
key of `is_serial`, which controls whether a metatask is parallel or
serial using pre-existing capability in rocoto. The default when not
given is parallel (i.e. most metatasks).

Resolves NOAA-EMC#2274
Refs NOAA-EMC/wxflow#39
DavidHuber-NOAA pushed a commit to NOAA-EMC/global-workflow that referenced this pull request Aug 12, 2024
Adds the ability to run a forecast in segments instead of all at once.
To accomplish this, a new local `checkpnts` variable is introduced to
`config.base` to contain a comma-separated list of intermediate stopping
points for the forecast. This is combined with `FHMIN_GFS` and
`FHMAX_GFS` to create a comma-separated string `FCST_SEGMENTS` with all
the start/end points that is used by `config.fcst` and rocoto workflow.
Capability to parse these into python lists was added to wxflow in an
accompanying PR. If `checkpnts` is an empty string, this will result in
a single-segment forecast.

To accommodate the new segment metatasks that must be run serially, the
capability of `create_task()` was expanded to allow a dictionary key of
`is_serial`, which controls whether a metatask is parallel or serial
using pre-existing capability in rocoto. The default when not given is
parallel (i.e. most metatasks).

Resolves #2274
Refs NOAA-EMC/wxflow#39
Refs NOAA-EMC/wxflow#40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants