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.

Sign up for GitHub

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

Jump to bottom

Fix test errors and type checking errors induced by a previous PR. #194

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix test errors and type checking errors induced by a previous PR. #194

wants to merge 1 commit into from

Conversation

fengxizhou
Copy link
Contributor

@fengxizhou fengxizhou commented Oct 23, 2024 •
edited
Loading

What does this PR do?

Fixes # 193 - Wrong Configuration Path Caused Test Failures

This PR fixes four things in a previous diff.

(1) A wrong file path for event_args_formats caused test errors. The fix is to update to the correct path.
(2) A type checking error caused by Library stubs not installed for "yaml". The fix is to add the following lines into pyproject.toml file.

[[tool.mypy.overrides]]
module = "yaml"
ignore_missing_imports = true

(3) Update mypy setting in .pre-commit-config.yaml to workaround errors: "Source file found twice under different module names"

(4) Fix format warnings when importing sorces fbcode into github.

Before submitting

  • [ x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
    • N/A
  • Did you write any new necessary tests?
    • [x ] N/A
  • Did you make sure to update the docs?
    • [x ] N/A
  • Did you update the changelog?
    • N/A

@fengxizhou fengxizhou linked an issue Oct 23, 2024 that may be closed by this pull request
BUG: Test Failures Caused By Wrong parse_event_args_yaml Path #193
Open
@fengxizhou fengxizhou self-assigned this Oct 23, 2024
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 23, 2024
sraikund16
sraikund16 reviewed Oct 23, 2024
View reviewed changes
hta/configs/event_args_yaml_parser.py
f"event_args_{version.get_version_str()}.yaml",
)
)
pkg_path: Path = Path(__file__).parent
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to get package path this way instead of using importlib.resources.files(package)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both should work, but HTA has been using the same method from the start.

briancoutinho
briancoutinho approved these changes Oct 23, 2024
View reviewed changes
Copy link
Contributor

@briancoutinho briancoutinho left a comment

Choose a reason for hiding this comment

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

lgtm :)

@facebook-github-bot
Copy link
Contributor

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

facebook-github-bot pushed a commit that referenced this pull request Oct 24, 2024
@fengxizhou @facebook-github-bot
Fix test errors and type checking errors induced by a previous PR. (#194
39c169b 
)

Summary:
## What does this PR do?
Fixes # 193 - Wrong Configuration Path Caused Test Failures

This PR fixes four things in a previous diff.

(1) A wrong file path for event_args_formats caused test errors. The fix is to update to the correct path.
(2) A type checking error caused by Library stubs not installed for "yaml". The fix is to add the following lines into `pyproject.toml` file.
```
[[tool.mypy.overrides]]
module = "yaml"
ignore_missing_imports = true
```
(3) Update mypy setting in .pre-commit-config.yaml to workaround errors: "Source file found twice under different module names"

(4) Fix format warnings when importing sorces fbcode into github.

## Before submitting

- [ x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
  - [ ] N/A
- [ ] Did you write any new necessary tests?
  - [x ] N/A
- [ ] Did you make sure to update the docs?
  - [x ] N/A
- [x] Did you update the [changelog](https://github.com/facebookresearch/HolisticTraceAnalysis/blob/main/CHANGELOG.md)?
  - [ ] N/A


Reviewed By: briancoutinho, sraikund16

Differential Revision: D64839500

Pulled By: fengxizhou
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D64839500

@fengxizhou @facebook-github-bot
Fix test errors and type checking errors induced by a previous PR. (#194
0921d22 
)

Summary:
## What does this PR do?
Fixes # 193 - Wrong Configuration Path Caused Test Failures

This PR fixes four things in a previous diff.

(1) A wrong file path for event_args_formats caused test errors. The fix is to update to the correct path.
(2) A type checking error caused by Library stubs not installed for "yaml". The fix is to add the following lines into `pyproject.toml` file.
```
[[tool.mypy.overrides]]
module = "yaml"
ignore_missing_imports = true
```
(3) Update mypy setting in .pre-commit-config.yaml to workaround errors: "Source file found twice under different module names"

(4) Fix format warnings when importing sorces fbcode into github.

## Before submitting

- [ x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
  - [ ] N/A
- [ ] Did you write any new necessary tests?
  - [x ] N/A
- [ ] Did you make sure to update the docs?
  - [x ] N/A
- [x] Did you update the [changelog](https://github.com/facebookresearch/HolisticTraceAnalysis/blob/main/CHANGELOG.md)?
  - [ ] N/A


Reviewed By: briancoutinho, sraikund16

Differential Revision: D64839500

Pulled By: fengxizhou
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D64839500

niufei8888 pushed a commit to niufei8888/HolisticTraceAnalysis that referenced this pull request Oct 24, 2024
@fengxizhou @facebook-github-bot
Fix test errors and type checking errors induced by a previous PR. (f…
dca1648 
…acebookresearch#194)

Summary:
## What does this PR do?
Fixes # 193 - Wrong Configuration Path Caused Test Failures

This PR fixes four things in a previous diff.

(1) A wrong file path for event_args_formats caused test errors. The fix is to update to the correct path.
(2) A type checking error caused by Library stubs not installed for "yaml". The fix is to add the following lines into `pyproject.toml` file.
```
[[tool.mypy.overrides]]
module = "yaml"
ignore_missing_imports = true
```
(3) Update mypy setting in .pre-commit-config.yaml to workaround errors: "Source file found twice under different module names"

(4) Fix format warnings when importing sorces fbcode into github.

## Before submitting

- [ x] Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
  - [ ] N/A
- [ ] Did you write any new necessary tests?
  - [x ] N/A
- [ ] Did you make sure to update the docs?
  - [x ] N/A
- [x] Did you update the [changelog](https://github.com/facebookresearch/HolisticTraceAnalysis/blob/main/CHANGELOG.md)?
  - [ ] N/A

Pull Request resolved: facebookresearch#194

Differential Revision:
D64839500

Privacy Context Container: L1200110

Reviewed By: briancoutinho, sraikund16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sraikund16 sraikund16 sraikund16 approved these changes

@briancoutinho briancoutinho briancoutinho approved these changes

Assignees

@fengxizhou fengxizhou

Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: Test Failures Caused By Wrong parse_event_args_yaml Path
4 participants
@fengxizhou @facebook-github-bot @briancoutinho @sraikund16