Skip to content

Use f-strings in torch.utils.cpp_extension#47025

Closed
malfet wants to merge 3 commits intopytorch:masterfrom
malfet:malfet/cpp-utils-f-string-refactor
Closed

Use f-strings in torch.utils.cpp_extension#47025
malfet wants to merge 3 commits intopytorch:masterfrom
malfet:malfet/cpp-utils-f-string-refactor

Conversation

@malfet
Copy link
Copy Markdown
Contributor

@malfet malfet commented Oct 28, 2020

Plus two minor fixes to torch/csrc/Module.cpp:

  • Use iterator of type Py_ssize_t for array indexing in THPModule_initNames
  • Fix clang-tidy warning of unneeded defaultGenerator copy by capturing it as const auto&

@malfet malfet added better-engineering Relatively self-contained tasks for better engineering contributors triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Oct 28, 2020
@malfet malfet requested a review from a team October 28, 2020 22:17
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@malfet has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@malfet malfet mentioned this pull request Oct 28, 2020
Copy link
Copy Markdown
Contributor

@samestep samestep left a comment

Choose a reason for hiding this comment

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

lgtm 👍 is there a reason these two files are both being changed in the same PR though?

@dr-ci
Copy link
Copy Markdown

dr-ci bot commented Oct 28, 2020

💊 CI failures summary and remediations

As of commit 65b4612 (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

ci.pytorch.org: 1 failed


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 1 time.

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 29, 2020

Codecov Report

Merging #47025 into master will not change coverage.
The diff coverage is 51.21%.

@@           Coverage Diff           @@
##           master   #47025   +/-   ##
=======================================
  Coverage   68.94%   68.94%           
=======================================
  Files         434      434           
  Lines       56189    56189           
=======================================
  Hits        38741    38741           
  Misses      17448    17448           

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@malfet merged this pull request in 42a5114.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

better-engineering Relatively self-contained tasks for better engineering contributors Merged triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants