Skip to content

Add RepeatDateTimeList#98

Open
gmertes wants to merge 13 commits into
ecmwf:developfrom
gmertes:feat/repeat-datetimelist
Open

Add RepeatDateTimeList#98
gmertes wants to merge 13 commits into
ecmwf:developfrom
gmertes:feat/repeat-datetimelist

Conversation

@gmertes

@gmertes gmertes commented May 27, 2026

Copy link
Copy Markdown
Member

Description

The next ecflow release 5.17.0 will have a new RepeatDateTimeList. It creates a repeat from any list of strings in yyyymmddTHHMMSS format.

The difference with the existing RepeatDateTime is that the list doesn't need to be continuous nor uniform, offering more flexibility.

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds support in pyflow for the upcoming ecFlow 5.17.0 RepeatDateTimeList attribute, enabling repeat definitions driven by an explicit (non-uniform / non-contiguous) list of datetimes.

Changes:

  • Introduces RepeatDateTimeList attribute class in pyflow/attributes.py.
  • Builds an ecflow.RepeatDateTimeList from a Python list of datetime/date/string inputs formatted as YYYYMMDDTHHMMSS.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pyflow/attributes.py
Comment thread pyflow/attributes.py
Comment thread pyflow/attributes.py
@marcosbento marcosbento force-pushed the feat/repeat-datetimelist branch 4 times, most recently from d701b31 to eb09295 Compare June 4, 2026 15:49

@marcosbento marcosbento left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After latest changes, it looks good to me!

@marcosbento

Copy link
Copy Markdown
Contributor

@gmertes , @corentincarton I have gone ahead and fixed the issues raised by Copilot and added a few tests to ensure everything is working. I believe it is ready to merge.

@marcosbento marcosbento marked this pull request as ready for review June 4, 2026 15:54
@colonesej colonesej requested a review from corentincarton June 4, 2026 15:57
@marcosbento

Copy link
Copy Markdown
Contributor

@corentincarton There is still an open issue, regarding how pyflow should detect/handle ecflow versions prior to 5.17.0 which don't have the new RepeatDateTimeList.

I believe we didn't made any special precautions when RepeatDateTime was introduced either -- correct me if I am wrong. Let me know what you think.

@colonesej

Copy link
Copy Markdown
Collaborator

regarding the ecflow version dependant features, could a solution be we create a decorator in pyflow that receives a version expression string (">=5.17") that would check the system ecflow version whenever that function/class is used and raise an error with appropriate message if it isn't supported?

pyflow already checks for ecflow client presence at runtime, it could also store its version in memory to carry out such checks

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento

Copy link
Copy Markdown
Contributor

@corentincarton Fixed the documentation build failure, but that implied touching the .github/workflows/readthedocs-pr.yml, to replace pull_request with pull_request_target, as per the action docs.

Unfortunately, this change combined with the fact that this is a PR from @gmertes' fork, triggered the "[PR Workflow Check]" which implies a human needs to review the changes.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento marcosbento force-pushed the feat/repeat-datetimelist branch from 6c9a349 to d14aabc Compare June 9, 2026 08:05
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento marcosbento force-pushed the feat/repeat-datetimelist branch from d14aabc to 450297c Compare June 9, 2026 08:06
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento marcosbento force-pushed the feat/repeat-datetimelist branch from 450297c to bad83cd Compare June 9, 2026 08:09
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento

Copy link
Copy Markdown
Contributor

@colonesej I just pushed the new class decorator supported_since that throws a NotImplementedError based on the current ecflow.__version__, and applied it to both RepeatDateTime and RepeatDateTimeList.
I placed the new decorator in importer.py since it is where ecflow is loaded, but I am not sure that is the right place for it -- this is very much an internal implementation detail, so maybe it should be hidden somehow?!. Please have a look.

@marcosbento marcosbento force-pushed the feat/repeat-datetimelist branch from bad83cd to 345011c Compare June 9, 2026 08:56
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento marcosbento requested a review from colonesej June 9, 2026 09:00
@colonesej

Copy link
Copy Markdown
Collaborator

@colonesej I just pushed the new class decorator supported_since that throws a NotImplementedError based on the current ecflow.__version__, and applied it to both RepeatDateTime and RepeatDateTimeList. I placed the new decorator in importer.py since it is where ecflow is loaded, but I am not sure that is the right place for it -- this is very much an internal implementation detail, so maybe it should be hidden somehow?!. Please have a look.

It looks good to me @marcosbento . we already fail at import if ecflow is not available so I don't think that being in importer is a problem. The name may be a bit off, but I don't think that's a major issue. One point/question about the _since concept: Isn't it useful to have a more "generic" expression or ecflow is never meant to deprecate anything?

@marcosbento marcosbento force-pushed the feat/repeat-datetimelist branch from 345011c to 714c0ee Compare June 12, 2026 13:15
@github-actions

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento marcosbento force-pushed the feat/repeat-datetimelist branch from 714c0ee to f842645 Compare June 12, 2026 14:22
@github-actions

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento marcosbento force-pushed the feat/repeat-datetimelist branch from f842645 to d16cc03 Compare June 12, 2026 14:24
@github-actions

Copy link
Copy Markdown

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@marcosbento

Copy link
Copy Markdown
Contributor

@corentincarton , @colonesej , @gmertes All done from my side... This one is waiting for someone to push the button!

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.

4 participants