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
Reproducibility Feature by Adding python_version
#172
Conversation
@sepandhaghighi please take a look at this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sadrasabouri Please take a look at my comments 💯
And also I suggest adding a test to overall_test.py and checking python_version
, something like this:
>>> g.python_version == sys.version.split()[0]
True
samila/genimage.py
Outdated
self.matplotlib_version), | ||
VERSION_WARNING.format( | ||
self.matplotlib_version, | ||
get_python_version()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be self.python_version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right
test/warning_test.py
Outdated
@@ -13,12 +13,20 @@ | |||
>>> g_.data2 == g.data2 | |||
True | |||
>>> with open('data.json', 'w') as fp: | |||
... json.dump({'data1': [0], 'data2': [0], 'python_version': '0'}, fp) | |||
>>> with warns(RuntimeWarning, match=r"Source matplotlib version(.*) or Python version(.*) is different from yours, plots may be different."): | |||
... g = GenerativeImage(lambda x,y: 0, lambda x,y: 0, data=open('data.json', 'r')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
g = GenerativeImage(data=open('data.json', 'r'))
test/warning_test.py
Outdated
... json.dump({'data1': [0], 'data2': [0], 'matplotlib_version': '0'}, fp) | ||
>>> with warns(RuntimeWarning, match=r"Source matplotlib version(.*) is different from yours, plots may be different."): | ||
>>> with warns(RuntimeWarning, match=r"Source matplotlib version(.*) or Python version(.*) is different from yours, plots may be different."): | ||
... g = GenerativeImage(lambda x,y: 0, lambda x,y: 0, data=open('data.json', 'r')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
g = GenerativeImage(data=open('data.json', 'r'))
Codecov Report
@@ Coverage Diff @@
## dev #172 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 575 587 +12
Branches 88 90 +2
=========================================
+ Hits 575 587 +12
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Reference Issues/PRs
#171
What does this implement/fix? Explain your changes.
This PR added the
python_version
attributes to theGenerativeImage
object which is saved in both config and data files. At the same time, loading these files Python version would be checked to see if it's the same as the current Python version.