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

[DO Not Merge] Avoid using 'default' keyword while invoking 'get' method #473

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leleamol
Copy link
Contributor

@leleamol leleamol commented Mar 24, 2021

Description of changes:

As mentioned in the issue #418 customers may see the error because of having 'default' keyword in the 'get()' call.

This change avoid using this keyword argument in 'get()' call to avoid potential issues.

As mentioned in the issue, the tokenizer in transformers package is modifying the data type of os.environ from os.Environ to 'dict'. The change in this PR is for protecting smdebug from such changes while keeping the behavior same.

Style and formatting:

I have run pre-commit install to ensure that auto-formatting happens with every commit.

Issue number, if available

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-io
Copy link

codecov-io commented Mar 24, 2021

Codecov Report

Merging #473 (f9b09ae) into master (1aac94b) will decrease coverage by 0.44%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #473      +/-   ##
==========================================
- Coverage   65.53%   65.09%   -0.45%     
==========================================
  Files         173      163      -10     
  Lines       13280    12934     -346     
==========================================
- Hits         8703     8419     -284     
+ Misses       4577     4515      -62     
Impacted Files Coverage Δ
smdebug/core/logger.py 70.83% <100.00%> (ø)
smdebug/core/modes.py 55.00% <0.00%> (-20.00%) ⬇️
smdebug/xgboost/singleton_utils.py 0.00% <0.00%> (-20.00%) ⬇️
smdebug/trials/profiler_trial.py 24.61% <0.00%> (-18.47%) ⬇️
smdebug/mxnet/collection.py 73.33% <0.00%> (-16.67%) ⬇️
smdebug/exceptions.py 65.47% <0.00%> (-15.48%) ⬇️
smdebug/core/reader.py 85.18% <0.00%> (-7.41%) ⬇️
smdebug/tensorflow/callable_cache.py 78.26% <0.00%> (-6.53%) ⬇️
smdebug/core/locations.py 86.11% <0.00%> (-5.56%) ⬇️
smdebug/core/tensor.py 79.03% <0.00%> (-2.02%) ⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1aac94b...f9b09ae. Read the comment docs.

@NihalHarish
Copy link
Contributor

Let's not merge this.
If this is the correct way to handle env variables then be it.

@leleamol leleamol changed the title Avoid using 'default' keyword while invoking 'get' method [DO Not Merge] Avoid using 'default' keyword while invoking 'get' method Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants