argparse migration for cmsDriver and FWCore #43042

kpedro88 · 2023-10-17T16:21:44Z

PR description:

See #29338. optparse has been deprecated for many years. argparse offers many functionality improvements, including (relevant to this PR in particular):

better type enforcement
automatic handling of exclusive options
named positional arguments
automatic formatting of usage strings
automatic inclusion of default values in help messages
handling options with different numbers of allowed arguments

While I cannot personally take on migrating every script in CMSSW, at least our most-used common tools should reflect best practices.

In addition, in the course of doing this migration, I fixed various typos and a few stray python3 migration issues.

PR validation:

Ran each script to ensure it still works. Unit tests pass.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Unlikely to be backported.

cmsbuild · 2023-10-17T16:27:58Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43042/37228

This PR adds an extra 72KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Applications/python/ConfigBuilder.py modified in PR(s): Introduce autoNANO #42238

cmsbuild · 2023-10-17T16:28:22Z

A new Pull Request was created by @kpedro88 (Kevin Pedro) for master.

It involves the following packages:

Configuration/Applications (operations)
FWCore/ParameterSet (core)
FWCore/PythonUtilities (core)
FWCore/Utilities (core)

@makortel, @antoniovilela, @smuzaffar, @davidlange6, @fabiocos, @cmsbuild, @Dr15Jones, @rappoccio can you please review it and eventually sign? Thanks.
@felicepantaleo, @makortel, @missirol, @Martin-Grunewald, @wddgit, @fabiocos this is something you requested to watch as well.
@rappoccio, @sextonkennedy, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2023-10-17T16:59:18Z

Configuration/Applications/python/cmsDriverOptions.py

+    from Configuration.Applications.Options import parser
+
+    options = parser.parse_args(items)
+    print(options)


was this just for debugging?

yes, let me just rebase that right out of there...

cmsbuild · 2023-10-17T17:08:09Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43042/37231

This PR adds an extra 72KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Applications/python/ConfigBuilder.py modified in PR(s): Introduce autoNANO #42238

cmsbuild · 2023-10-17T17:08:37Z

Pull request #43042 was updated. @antoniovilela, @davidlange6, @Dr15Jones, @fabiocos, @makortel, @rappoccio, @smuzaffar, @cmsbuild can you please check and sign again.

Dr15Jones · 2023-10-17T17:07:18Z

FWCore/ParameterSet/test/comparePythonOutput.py

@@ -4,15 +4,18 @@
 #  dictionaries or escape characters

 from __future__ import print_function


could this line go away?

Dr15Jones · 2023-10-17T17:07:35Z

FWCore/PythonUtilities/scripts/compareJSON.py

@@ -2,54 +2,55 @@

 from __future__ import print_function


Dr15Jones · 2023-10-17T17:08:27Z

FWCore/PythonUtilities/scripts/csv2json.py

@@ -2,30 +2,27 @@

 from __future__ import print_function


Dr15Jones · 2023-10-17T17:08:49Z

FWCore/PythonUtilities/scripts/filterCSVwithJSON.py

@@ -2,34 +2,34 @@

 from __future__ import print_function


Dr15Jones · 2023-10-17T17:08:59Z

FWCore/PythonUtilities/scripts/filterJSON.py

@@ -2,27 +2,25 @@

 from __future__ import print_function


Dr15Jones · 2023-10-17T17:09:25Z

Configuration/Applications/scripts/cmsDriver.py

@@ -14,17 +13,6 @@ def run():
        # after cleanup of all config parameters pass it to the ConfigBuilder
        configBuilder = ConfigBuilder(options, with_output = True, with_input = True)

-        # Switch on any eras that have been specified. This is not required to create


Where did the era handling go? Or was this not actually needed?

It was obsoleted by #39127.

Dr15Jones · 2023-10-17T17:11:10Z

FWCore/PythonUtilities/scripts/printJSON.py

@@ -2,21 +2,18 @@

 from __future__ import print_function


Dr15Jones · 2023-10-17T17:11:17Z

FWCore/PythonUtilities/scripts/mergeJSON.py

@@ -2,11 +2,10 @@

 from __future__ import print_function


Dr15Jones · 2023-10-17T17:12:48Z

FWCore/Utilities/scripts/edmCheckClassVersion

@@ -1,6 +1,5 @@
 #!  /usr/bin/env python3
 from __future__ import print_function


Dr15Jones · 2023-10-17T17:14:46Z

I wouldn't have commented about the from __future__ line except you did remove it from one of the files.

cmsbuild · 2023-10-17T21:29:10Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43042/37238

This PR adds an extra 76KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Applications/python/ConfigBuilder.py modified in PR(s): Introduce autoNANO #42238

cmsbuild · 2023-10-19T08:46:05Z

Pull request #43042 was updated. @makortel, @smuzaffar, @Dr15Jones, @rappoccio, @davidlange6, @antoniovilela, @cmsbuild, @fabiocos can you please check and sign again.

kpedro88 · 2023-10-19T08:49:20Z

please test

cmsbuild · 2023-10-19T11:21:13Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-84c5f6/35274/summary.html
COMMIT: d27742b
CMSSW: CMSSW_13_3_X_2023-10-18-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43042/35274/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 14 lines to the logs
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3357400
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3357375
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

makortel · 2023-10-24T16:25:18Z

+core

antoniovilela · 2023-10-25T01:46:58Z

+1

cmsbuild · 2023-10-25T01:47:24Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged.

cmsbuild added this to the CMSSW_13_3_X milestone Oct 17, 2023

cmsbuild added core-pending operations-pending pending-signatures tests-pending orp-pending code-checks-pending labels Oct 17, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 17, 2023

Dr15Jones reviewed Oct 17, 2023

View reviewed changes

kpedro88 added 4 commits October 17, 2023 12:02

migrate cmsDriver to argparse

5a3363f

argparse migration for FWCore/ParameterSet

c805714

argparse migration for FWCore/PythonUtilities

907b15e

argparse migration for FWCore/Utilities (& some python3 fixes)

19102b3

kpedro88 force-pushed the CoreArgParse branch from fa5a1e4 to 19102b3 Compare October 17, 2023 17:03

cmsbuild added code-checks-pending and removed code-checks-approved labels Oct 17, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 17, 2023

Dr15Jones reviewed Oct 17, 2023

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-approved labels Oct 17, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 17, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 19, 2023

cmsbuild added tests-started and removed tests-pending labels Oct 19, 2023

cmsbuild added tests-approved and removed tests-started labels Oct 19, 2023

kpedro88 mentioned this pull request Oct 24, 2023

argparse migration for PhysicsTools and FWLite #43102

Merged

cmsbuild added core-approved and removed core-pending labels Oct 24, 2023

cmsbuild added operations-approved fully-signed orp-approved and removed operations-pending pending-signatures orp-pending labels Oct 25, 2023

cmsbuild merged commit ba0d01e into cms-sw:master Oct 25, 2023

This was referenced Oct 25, 2023

Update the Phase-2 L1 vertex tag in the P2GT producer #43109

Merged

Backport jitlink ppc64 cms-sw/root#188

Closed

Add DQMOnlineBeamspot streamer files for run 370580 cms-data/DQM-Integration#6

Merged

kpedro88 mentioned this pull request Oct 25, 2023

fix cmsDriver arguments in testTauEmbeddingProducers #43116

Merged

This was referenced Oct 25, 2023

Update beamhlt client unitTests #43110

Merged

Updated root to tip of branch master cms-sw/cmsdist#8777

Merged

kpedro88 mentioned this pull request Jan 3, 2024

Make cmsRun-compatible argparse parser #34469

Closed

germanfgv mentioned this pull request Sep 23, 2024

Update data processing tests generation Tier-0 configurations #46103

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

argparse migration for cmsDriver and FWCore #43042

argparse migration for cmsDriver and FWCore #43042

kpedro88 commented Oct 17, 2023

cmsbuild commented Oct 17, 2023

cmsbuild commented Oct 17, 2023

Dr15Jones Oct 17, 2023

kpedro88 Oct 17, 2023

cmsbuild commented Oct 17, 2023

cmsbuild commented Oct 17, 2023

Dr15Jones Oct 17, 2023

Dr15Jones Oct 17, 2023

Dr15Jones Oct 17, 2023

Dr15Jones Oct 17, 2023

Dr15Jones Oct 17, 2023

Dr15Jones Oct 17, 2023

kpedro88 Oct 17, 2023

Dr15Jones Oct 17, 2023

Dr15Jones Oct 17, 2023

Dr15Jones Oct 17, 2023

Dr15Jones commented Oct 17, 2023

cmsbuild commented Oct 17, 2023

cmsbuild commented Oct 19, 2023

kpedro88 commented Oct 19, 2023

cmsbuild commented Oct 19, 2023

makortel commented Oct 24, 2023

antoniovilela commented Oct 25, 2023

cmsbuild commented Oct 25, 2023

		@@ -4,15 +4,18 @@
		# dictionaries or escape characters

		from __future__ import print_function

		@@ -1,6 +1,5 @@
		#! /usr/bin/env python3
		from __future__ import print_function

argparse migration for cmsDriver and FWCore #43042

argparse migration for cmsDriver and FWCore #43042

Conversation

kpedro88 commented Oct 17, 2023

PR description:

PR validation:

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

cmsbuild commented Oct 17, 2023

cmsbuild commented Oct 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Oct 17, 2023

cmsbuild commented Oct 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dr15Jones commented Oct 17, 2023

cmsbuild commented Oct 17, 2023

cmsbuild commented Oct 19, 2023

kpedro88 commented Oct 19, 2023

cmsbuild commented Oct 19, 2023

Comparison Summary

makortel commented Oct 24, 2023

antoniovilela commented Oct 25, 2023

cmsbuild commented Oct 25, 2023