Skip to content

Format source code to PEP8 using Ruff - #36

Merged
jamessiddeley-amd merged 24 commits into
developfrom
import/develop/jamessiddeley-amd_rocprofiler-compute/jsiddele_pre-commit-hooks
Aug 8, 2025
Merged

jamessiddeley-amd merged 24 commits into
developfrom
import/develop/jamessiddeley-amd_rocprofiler-compute/jsiddele_pre-commit-hooks

Conversation

@systems-assistant

@systems-assistant systems-assistant Bot commented Aug 6, 2025 •

Copy link
Copy Markdown
Contributor

Changed rocprof pre-commit hooks to use ruff instead of isort and black. Additional manual changes to current rocprof python files to match PEP8 format.

fix: Correct syntax error in multi-line f-string

Addresses a syntax error in src/rocprof_compute_tui/views/main_view.py
that was introduced during a manual code formatting pass.

Ruff correctly identified a line as being too long but was unable to
apply an automatic fix. The subsequent manual edit incorrectly
added a newline character inside an f-string, causing the error.

The issue was an isolated manual error and not a
systemic problem with the new ruff/black formatting setup.

Relates to #315

🔁 Imported from ROCm/rocprofiler-compute#713
🧑‍💻 Originally authored by @jamessiddeley-amd

@vedithal-amd vedithal-amd 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.

LGTM

)
'''
[tool.ruff]
target-version="py38"

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.

I'd say we should remove the target-version

@feizheng10

feizheng10 commented Aug 6, 2025 •

Copy link
Copy Markdown
Contributor

I recall we had conversion something like adding guidance in readme:
There should be a chapter for contribution guide lines: how to install and run ruff before submit PR. And also mention that how to disable formatting code with unwanted sections.

@feizheng10 feizheng10 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.

LGTM with minor comments.
Please run the whole test suites locally to make sure all tests passed.

@jamessiddeley-amd

Copy link
Copy Markdown
Contributor

LGTM with minor comments. Please run the whole test suites locally to make sure all tests passed.

Just ran on latest dev:

Test project /app/build
      Start  1: test_profile_kernel_execution
 1/16 Test  #1: test_profile_kernel_execution ....   Passed   46.11 sec
      Start  2: test_profile_dispatch
 2/16 Test  #2: test_profile_dispatch ............   Passed  131.29 sec
      Start  3: test_profile_mem
 3/16 Test  #3: test_profile_mem .................   Passed  175.43 sec
      Start  4: test_profile_join
 4/16 Test  #4: test_profile_join ................   Passed   88.03 sec
      Start  5: test_profile_sort
 5/16 Test  #5: test_profile_sort ................   Passed  174.83 sec
      Start  6: test_profile_misc
 6/16 Test  #6: test_profile_misc ................   Passed  1105.42 sec
      Start  7: test_profile_section
 7/16 Test  #7: test_profile_section .............   Passed   56.17 sec
      Start  8: test_profile_sets_func
 8/16 Test  #8: test_profile_sets_func ...........   Passed   32.65 sec
      Start  9: test_analyze_commands
 9/16 Test  #9: test_analyze_commands ............   Passed  462.41 sec
      Start 10: test_analyze_workloads
10/16 Test #10: test_analyze_workloads ...........   Passed  709.25 sec
      Start 11: test_L1_cache_counters
11/16 Test #11: test_L1_cache_counters ...........   Passed   67.96 sec
      Start 12: test_num_xcds_spec_class
12/16 Test #12: test_num_xcds_spec_class .........   Passed    3.97 sec
      Start 13: test_num_xcds_cli_output
13/16 Test #13: test_num_xcds_cli_output .........   Passed    4.78 sec
      Start 14: test_db_connector
14/16 Test #14: test_db_connector ................   Passed    2.37 sec
      Start 15: test_utils
15/16 Test #15: test_utils .......................   Passed   11.52 sec
      Start 16: test_autogen_config
16/16 Test #16: test_autogen_config ..............   Passed    2.19 sec

100% tests passed, 0 tests failed out of 16

Label Time Summary:
profile    = 1721.10 sec*proc (6 tests)

Total Test time (real) = 3074.38 sec

ammallya pushed a commit that referenced this pull request Aug 7, 2025
* Adding tool pc sampling support

Fixing merge issue

tool support on SDKupdates

link amd-comgr

Sanitizer failure fix

fix format

Addressing review comments

misc fix

Adding dispatch id to the CSV output

AddingCHANGELOG

[ROCProfV3][PC Sampling] Initial ROCProfV3 PC sampling tests for JSON and CSV formats (#17)

ROCProfV3 initial tests for JSON and CSV output.

Simple kernels that simplify the verification of samples to instruction decoding
has been introduced.

removing option to enable pc sampling explicitly

Adding documentation

no pc-sampling option in tests anymore

Addressing review comments

Updating docs

an option for choosing whether all units must be sampled

try ignoring PC sampling tests (#36)

* run pc-sampling tests on MI2xx runners
* use v_fmac_f32 instead of s_nop 0 in tests

* fixing docs

[ROCm/rocprofiler-sdk commit: 50b185b]
@jamessiddeley-amd
jamessiddeley-amd merged commit 58d2a01 into develop Aug 8, 2025
@jamessiddeley-amd
jamessiddeley-amd deleted the import/develop/jamessiddeley-amd_rocprofiler-compute/jsiddele_pre-commit-hooks branch August 8, 2025 19:32
systems-assistant Bot pushed a commit to ROCm/rocprofiler-compute that referenced this pull request Aug 8, 2025
* added ruff docs

* style: Run ruff and black before yapf pass

* yapf -r -i (23 fixes)

* fixed conf.py and ran ruff format .

* fixed conf.py 2

* formatted argparser.py

* formatted src/rocprof_compute_analyze

* formatted src/rocprof_compute_profile

* formatted soc_base.py

* formatted rocprof_compute_tui

* formatted gui_components

* formatted src/utils

* formatted tests/

* format extra files

* cleanup

* fix test_utils.py

* fixed typos

* Update pyproject.toml

* Update README.md

* Update test_utils.py
[rocm-systems] ROCm/rocm-systems#36 (commit 58d2a01)
systems-assistant Bot pushed a commit that referenced this pull request Aug 10, 2025
@jamessiddeley-amd

jamessiddeley-amd commented Aug 12, 2025 •

Copy link
Copy Markdown
Contributor

fix: Correct syntax error in multi-line f-string

Addresses a syntax error in src/rocprof_compute_tui/views/main_view.py
that was introduced during a manual code formatting pass.

Ruff correctly identified a line as being too long but was unable to
apply an automatic fix. The subsequent manual edit incorrectly
added a newline character inside an f-string, causing the error.

The issue was an isolated manual error and not a
systemic problem with the new ruff/black formatting setup.

Relates to #315

image

jayhawk-commits pushed a commit that referenced this pull request Aug 18, 2025
#36)

SWDEV-546293 - Adds nvidia mappings to hipMemPrefetchAsync_vs and hipMemAdvise_v2
jayhawk-commits pushed a commit that referenced this pull request Aug 18, 2025
jayhawk-commits pushed a commit that referenced this pull request Aug 18, 2025
#36)

SWDEV-546293 - Adds nvidia mappings to hipMemPrefetchAsync_vs and hipMemAdvise_v2

[ROCm/hipother commit: bf0302a]
ammallya pushed a commit that referenced this pull request Jan 21, 2026
Add cooperative groups for sync collective
ammallya pushed a commit that referenced this pull request Jan 21, 2026
…m-sync

Fix `rocshmem_ctx_wg_team_sync` API
ammallya pushed a commit that referenced this pull request Jan 21, 2026
Add cooperative groups for sync collective

[ROCm/rocshmem commit: 8015a45]
ammallya pushed a commit that referenced this pull request Jan 21, 2026
…m-sync

Fix `rocshmem_ctx_wg_team_sync` API

[ROCm/rocshmem commit: d97e5ba]
ammallya pushed a commit that referenced this pull request Apr 9, 2026
As things get more complicated, the install instructions will become too
complicated to fit in README.md. This also seems to be where TheRock
components seems to put their install instructions.
ammallya pushed a commit that referenced this pull request Apr 9, 2026
As things get more complicated, the install instructions will become too
complicated to fit in README.md. This also seems to be where TheRock
components seems to put their install instructions.

[ROCm/hipFile commit: 5c97904]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants