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

[py2py3] Migration at level scr/python/A/B/C - slice 2 #10103

Merged
merged 2 commits into from
Dec 11, 2020

Conversation

mapellidario
Copy link
Member

@mapellidario mapellidario commented Nov 24, 2020

Fixes #10098

Status

ready

Related PRs

This PR is rebased on master.

Description

Run futurize and some manual changes on the first batch of src/python/A/B/C.

No notable changes or errors have been encountered during this PR.

Is it backward compatible (if not, which system it affects?)

It should be. No possible causes for errors have been spot by unit tests.

External dependencies / deployment changes

Requires python-future in both py2 and py3 environments.

@mapellidario mapellidario added Python3 PR: Do not merge yet PR: Work in progress py2py3 slice Issue related to slicing the modernization to py2py3 labels Nov 24, 2020
@mapellidario mapellidario self-assigned this Nov 24, 2020
@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 65 warnings and errors that must be fixed
    • 69 warnings
    • 393 comments to review
  • Pycodestyle check: succeeded
    • 462 comments to review
  • Python3 compatibility checks: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10813/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 73 warnings and errors that must be fixed
    • 85 warnings
    • 450 comments to review
  • Pycodestyle check: succeeded
    • 510 comments to review
  • Python3 compatibility checks: succeeded

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10818/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
    • 1 changes in unstable tests
  • Pylint check: failed
    • 79 warnings and errors that must be fixed
    • 86 warnings
    • 513 comments to review
  • Pycodestyle check: succeeded
    • 566 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10820/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 80 warnings and errors that must be fixed
    • 86 warnings
    • 537 comments to review
  • Pycodestyle check: succeeded
    • 612 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10821/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 84 warnings and errors that must be fixed
    • 86 warnings
    • 550 comments to review
  • Pycodestyle check: succeeded
    • 617 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10822/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: failed
    • 1 new failures
  • Pylint check: failed
    • 86 warnings and errors that must be fixed
    • 87 warnings
    • 568 comments to review
  • Pycodestyle check: succeeded
    • 671 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10823/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 86 warnings and errors that must be fixed
    • 87 warnings
    • 568 comments to review
  • Pycodestyle check: succeeded
    • 671 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10825/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 98 warnings and errors that must be fixed
    • 90 warnings
    • 632 comments to review
  • Pycodestyle check: succeeded
    • 725 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10826/artifact/artifacts/PullRequestReport.html

@mapellidario mapellidario force-pushed the py2py3-issue10098 branch 2 times, most recently from 47aa086 to 322c452 Compare November 26, 2020 10:12
@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 98 warnings and errors that must be fixed
    • 90 warnings
    • 632 comments to review
  • Pycodestyle check: succeeded
    • 725 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10827/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 98 warnings and errors that must be fixed
    • 90 warnings
    • 632 comments to review
  • Pycodestyle check: succeeded
    • 725 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10828/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 98 warnings and errors that must be fixed
    • 90 warnings
    • 632 comments to review
  • Pycodestyle check: succeeded
    • 725 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10837/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 98 warnings and errors that must be fixed
    • 90 warnings
    • 632 comments to review
  • Pycodestyle check: succeeded
    • 725 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10839/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 85 warnings and errors that must be fixed
    • 90 warnings
    • 633 comments to review
  • Pycodestyle check: succeeded
    • 721 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10840/artifact/artifacts/PullRequestReport.html

@mapellidario
Copy link
Member Author

mapellidario commented Dec 10, 2020

@amaltaro after discussion on slack, this has been rebased directly onto master and it is ready to be reviewed!

@mapellidario mapellidario force-pushed the py2py3-issue10098 branch 2 times, most recently from e1597cd to 48d3038 Compare December 10, 2020 14:14
@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
    • 1 tests no longer failing
    • 1 changes in unstable tests
  • Pylint check: failed
    • 31 warnings and errors that must be fixed
    • 21 warnings
    • 239 comments to review
  • Pylint py3k check: succeeded
    • 0 errors and warnings that should be fixed
    • 0 warnings
    • 0 comments to review
  • Pycodestyle check: succeeded
    • 263 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10924/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: failed
    • 4 tests deleted
    • 1 tests no longer failing
    • 1 tests added
    • 1 changes in unstable tests
  • Pylint check: failed
    • 32 warnings and errors that must be fixed
    • 21 warnings
    • 241 comments to review
  • Pylint py3k check: succeeded
    • 0 errors and warnings that should be fixed
    • 0 warnings
    • 0 comments to review
  • Pycodestyle check: succeeded
    • 263 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10929/artifact/artifacts/PullRequestReport.html

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
    • 1 tests no longer failing
    • 1 changes in unstable tests
  • Pylint check: failed
    • 31 warnings and errors that must be fixed
    • 21 warnings
    • 239 comments to review
  • Pylint py3k check: succeeded
    • 0 errors and warnings that should be fixed
    • 0 warnings
    • 0 comments to review
  • Pycodestyle check: succeeded
    • 263 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10930/artifact/artifacts/PullRequestReport.html

Copy link
Contributor

@amaltaro amaltaro left a comment

Choose a reason for hiding this comment

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

These changes look good to me, Dario. I assume they are ready to be merged as well?

src/python/WMCore/DAOFactory.py Outdated Show resolved Hide resolved
@@ -112,6 +108,6 @@ def __str__(self):
"""
string = ''
for i in self.create, self.constraints, self.inserts, self.indexes:
for j in i.keys():
for j in i:
Copy link
Contributor

Choose a reason for hiding this comment

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

such a horrible variable naming... nothing for you to do though!

@amaltaro
Copy link
Contributor

Actually, can you please squash commits 2 and 3?

@mapellidario
Copy link
Member Author

I squashed commit about test and pylint and added a commit with your suggestion about WMCore/DAOFactory.py.

After you give the ok, I can squash the commits about src and then this is ready to be merged

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 31 warnings and errors that must be fixed
    • 21 warnings
    • 239 comments to review
  • Pylint py3k check: succeeded
    • 0 errors and warnings that should be fixed
    • 0 warnings
    • 0 comments to review
  • Pycodestyle check: succeeded
    • 263 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10935/artifact/artifacts/PullRequestReport.html

@amaltaro
Copy link
Contributor

Thanks Dario. Feel free to squash the src changes and it's ready to go.

@cmsdmwmbot
Copy link

Jenkins results:

  • Unit tests: succeeded
  • Pylint check: failed
    • 31 warnings and errors that must be fixed
    • 21 warnings
    • 239 comments to review
  • Pylint py3k check: succeeded
    • 0 errors and warnings that should be fixed
    • 0 warnings
    • 0 comments to review
  • Pycodestyle check: succeeded
    • 263 comments to review
  • Python3 compatibility checks: succeeded
    • there are suggested fixes for newer python3 idioms

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/10939/artifact/artifacts/PullRequestReport.html

@amaltaro
Copy link
Contributor

Oups, I overlooked the python2.6 idioms. Have you tried updating those (at the very bottom of the PR report)? Or should we leave it for another PR?

@mapellidario
Copy link
Member Author

Since it is the usual type vs isisntance that broke JSONThunker a couple of months ago, i am a bit scared of touching those, especially when dicts are involved. I can give it a try and hope for the best, though

@amaltaro
Copy link
Contributor

@mapellidario Dario, I hope you haven't started working on this type x isinstance thing.
As suggested over slack, how about we deal - globally - with this type vs isinstance change in a different PR. I do not expect many modules to be changed and we can nicely isolate that from anything else, making our life easier to spot issues like the JSONThunker one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
py2py3 slice Issue related to slicing the modernization to py2py3 Python3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[py2py3] Migration at level scr/python/A/B/C - slice 2
3 participants