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

Require semantic version for model parameter version string #1103

Merged
merged 7 commits into from
Aug 29, 2024

Conversation

GernotMaier
Copy link
Contributor

@GernotMaier GernotMaier commented Aug 27, 2024

Model versions have changed to semantic versioning with this model_parameters PR.

Add here:

  • a test to validate_data that the version string follows semantic versioning.
  • change of "2024-02-01" to prod6 and "2020-06-28" to prod5
  • run validate_camera_efficiency_ssts test for prod6 only (as Derived-DB is not updated with the new versioning; see issue Review and replace derived value DB #1070)

@GernotMaier GernotMaier self-assigned this Aug 27, 2024

This comment has been minimized.

Copy link

Passed

Analysis Details

1 Issue

  • Bug 0 Bugs
  • Vulnerability 1 Vulnerability
  • Code Smell 0 Code Smells

Coverage and Duplications

  • Coverage 100.00% Coverage (91.10% Estimated after merge)
  • Duplications 0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: gammasim_simtools_AY_ssha9WiFxsX-2oy_w

View in SonarQube

"""
if version is None:
return
semver_regex = r"^\d+\.\d+\.\d+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not perfectly familiar with regex. Does this here allow also number.number.number-(additional expression)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - but see also #1115

"1.0.0",
"0.1.0",
"2.3.4",
"1.0.0-alpha",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Related to above question. So we really want to allow this kind of version structure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how semantic versioning is defined, see https://semver.org/

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok all good then.

Copy link
Collaborator

@tobiaskleiner tobiaskleiner left a comment

Choose a reason for hiding this comment

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

I put this comment here that this would also accept pre release versions.

@GernotMaier GernotMaier merged commit a7c89c1 into main Aug 29, 2024
19 checks passed
@GernotMaier GernotMaier deleted the model-parameters-semantic-versioning branch August 29, 2024 11:51
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.

2 participants