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

fix(launch): Default log_code exclusion behavior now correctly handles wandb in the root path prefix. #6095

Merged
merged 12 commits into from
Aug 18, 2023

Conversation

nickpenaranda
Copy link
Contributor

@nickpenaranda nickpenaranda commented Aug 17, 2023

Fixes

Description

What does the PR do?

🤖 Generated by Copilot at e4fef90

This pull request improves the file filtering and logging functionality in the wandb/sdk/lib and wandb/sdk modules, and adds unit tests for the filenames.py module. It allows users to customize the file inclusion and exclusion criteria for logging source code to the W&B dashboard, and makes the file filtering logic more robust and flexible.

More specifically, this addresses an issue that prevents log_code from behaving as expected when the absolute path to the root directory contains wandb or .wandb. This PR...

  1. expands log_code to allow callers to pass either 1- or 2-parameter callables for include_fn and/or exclude_fn, where the 1st argument is the absolute file path, and the 2nd argument is the absolute root path
  2. reimplements the default exclude_fn to exclude only <root>/.wandb/* and <root>/wandb/* as originally intended, and NOT all paths that contain .wandb or wandb anywhere.

Testing

How was this PR tested? New unit test suite

Checklist

  • Include reference to internal ticket "Fixes WB-NNNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
  • Ensure PR title compliance with the conventional commits standards

🤖 Generated by Copilot at e4fef90

To log code with W&B, you'll see
You can filter files with log_code
Use filtered_dir and exclude_wandb_fn
To customize what you want to send
And test your logic with test_filenames.py

@nickpenaranda nickpenaranda changed the title fix(artifacts): log_code works correctly in directories that contain 'wandb' fix(launch): log_code works correctly in directories that contain 'wandb' Aug 17, 2023
@github-actions github-actions bot added cc-fix and removed cc-fix labels Aug 17, 2023
@nickpenaranda nickpenaranda marked this pull request as ready for review August 17, 2023 03:04
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Merging #6095 (68fa9d0) into main (1771a8a) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6095      +/-   ##
==========================================
- Coverage   78.11%   78.02%   -0.09%     
==========================================
  Files         380      380              
  Lines       44236    44241       +5     
==========================================
- Hits        34555    34521      -34     
- Misses       9631     9670      +39     
  Partials       50       50              
Flag Coverage Δ
unittest 81.52% <100.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
wandb/sdk/wandb_run.py 91.10% <ø> (ø)
wandb/sdk/lib/filenames.py 100.00% <100.00%> (ø)

... and 13 files with indirect coverage changes

@github-actions github-actions bot added cc-fix and removed cc-fix labels Aug 17, 2023
@nickpenaranda nickpenaranda changed the title fix(launch): log_code works correctly in directories that contain 'wandb' fix(launch): Default log_code exclusion behavior now correctly handles wandb in the root path prefix. Aug 18, 2023
@github-actions github-actions bot added cc-fix and removed cc-fix labels Aug 18, 2023
@nickpenaranda nickpenaranda merged commit 95b6650 into main Aug 18, 2023
75 checks passed
@nickpenaranda nickpenaranda deleted the nick/fix-exclude-wandb branch August 18, 2023 16:33
@github-actions github-actions bot added cc-fix and removed cc-fix labels Jan 7, 2024
@kptkin kptkin added this to the sdk-2023-09.1 milestone Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants