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

Add Type Annotations #58

Merged
merged 11 commits into from
Oct 11, 2019
Merged

Add Type Annotations #58

merged 11 commits into from
Oct 11, 2019

Conversation

mhrownaghi
Copy link
Contributor

#55

@mhrownaghi mhrownaghi changed the title Add Type Annotation(config_tools.py) Add Type Annotation(config_tools.py & crystals.py) Oct 10, 2019
@mhrownaghi mhrownaghi changed the title Add Type Annotation(config_tools.py & crystals.py) Add Type Annotations Oct 10, 2019
@mhrownaghi mhrownaghi closed this Oct 11, 2019
@mhrownaghi mhrownaghi reopened this Oct 11, 2019
@dalonsoa
Copy link
Collaborator

Request my review on the top right corner when you're ready for me to check this and merge :)

mhrownaghi
mhrownaghi commented Oct 11, 2019
View reviewed changes
Copy link
Contributor Author

@mhrownaghi mhrownaghi left a comment

Choose a reason for hiding this comment

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

Review my commits.

dalonsoa
dalonsoa requested changes Oct 11, 2019
View reviewed changes
Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

Great job! Just a couple of minor corrections.

solcore/config_tools.py Outdated Show resolved Hide resolved
solcore/interpolate.py Outdated Show resolved Hide resolved
mhrownaghi
mhrownaghi commented Oct 11, 2019
View reviewed changes
Copy link
Contributor Author

@mhrownaghi mhrownaghi left a comment •
edited

Choose a reason for hiding this comment

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

I corrected config_tools.py and interpolate.py.

mhrownaghi
mhrownaghi commented Oct 11, 2019
View reviewed changes
Copy link
Contributor Author

@mhrownaghi mhrownaghi left a comment

Choose a reason for hiding this comment

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

I added type annotation to smooth.py. Please review it. Thanks

dalonsoa
dalonsoa approved these changes Oct 11, 2019
View reviewed changes
Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

Excellent work! Specially with the singleton. That was not an easy one! Just a correction to one of the annotations, which should be a Callable and not a str.

solcore/singleton.py Outdated Show resolved Hide resolved
solcore/singleton.py Outdated Show resolved Hide resolved
mhrownaghi and others added 3 commits October 11, 2019 19:09
@mhrownaghi @dalonsoa
Update solcore/singleton.py
425c0f1 
Co-Authored-By: Diego <d.alonso-alvarez@imperial.ac.uk>
@mhrownaghi @dalonsoa
Update solcore/singleton.py
8a34681 
Co-Authored-By: Diego <d.alonso-alvarez@imperial.ac.uk>
@dalonsoa
Merge branch 'devel' into devel
3e403c7
@dalonsoa dalonsoa merged commit 2d269cb into qpv-research-group:devel Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dalonsoa dalonsoa dalonsoa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mhrownaghi @dalonsoa