feat: Add store_full_path to converters (2/3) #8573
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
topic:tests
type:documentation
Pull Request Test Coverage Report for Build 11974098350Details
💛 - Coveralls |
Amnah199
requested review from
dfokina and
davidsbatista
and removed request for
a team
| for text, extra_meta in data: | ||
| merged_metadata = {**bytestream.meta, **metadata, **extra_meta} | ||
|
|
||
| if not self._store_full_path and "file_path" in bytestream.meta: | ||
| file_path = bytestream.meta.get("file_path") |
There was a problem hiding this comment.
you can do this with the walrus operator, merging if the assignment in one go
if file_path := bytestream.meta.get("file_path"):
merged_metadata["file_path"] = os.path.basename(file_path)|
|
||
| if not self.store_full_path and "file_path" in bytestream.meta: | ||
| file_path = bytestream.meta.get("file_path") | ||
| if file_path: # Ensure the value is not None for pylint |
|
|
||
| if not self.store_full_path and "file_path" in bytestream.meta: | ||
| file_path = bytestream.meta.get("file_path") | ||
| if file_path: # Ensure the value is not None for pylint |
|
|
||
| if not self.store_full_path and "file_path" in bytestream.meta: | ||
| file_path = bytestream.meta.get("file_path") | ||
| if file_path: # Ensure the value is not None for pylint |
|
|
||
| if not self.store_full_path and "file_path" in bytestream.meta: | ||
| file_path = bytestream.meta.get("file_path") | ||
| if file_path: # Ensure the value is not None for pylint |
|
|
||
| if not self.store_full_path and "file_path" in bytestream.meta: | ||
| file_path = bytestream.meta.get("file_path") | ||
| if file_path: # Ensure the value is not None for pylint |
|
|
||
| deprecations: | ||
| - | | ||
| The default value of the `store_full_path` parameter will be changed to `False` in Haysatck 2.9.0 to enhance privacy. |
There was a problem hiding this comment.
Suggested change
| The default value of the `store_full_path` parameter will be changed to `False` in Haysatck 2.9.0 to enhance privacy. | |
| The default value of the `store_full_path` parameter will change to `False` in Haysatck 2.9.0 to enhance privacy. |
|
@Amnah199 looks good, left a comment regarding small improvement |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issues
Proposed Changes:
This PR is similar to #8566
Add a new parameter to the
__init__method to allow users to toggle whether thefile_pathin metadata stores the complete file path or only the base file name. ADeprecationWarningis generated about the upcoming change infile_pathstorage behavior in the next release.How did you test it?
Added new test for checking the parameter function
CI
Notes for the reviewer
This is 2/3 PRs that will add the same change to other converters. PR has many files but they all include the same change.
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.