-
Notifications
You must be signed in to change notification settings - Fork 18
fix: import loadData
and deserialize_data
directly to adopt new API from diffpy.utils(3.6.0)
#162
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
Conversation
@sbillinge It's ready for review, thank you. |
This looks like a better solution - assuming local test and GH pass. |
@ycexiao also the PR title can be a bit clarified. The only use information in the title is "new" - we might want to add information that this PR directly imports loaddata and deserializae methods from diffpy.util 3.6.0 (also news should be clarified that it's an API update to be compatiable with diffpy.utils 3.6.0) |
parsers
apiloadData
and deserialize_data
directly to adopt new API from diffpy.utils(3.6.0)
Thank you. Details have been added to issue title and news |
Thanks @ycexiao , good changes! I like it. |
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.
Thanks @ycexiao . Code fixes look good (I didn't check tests pass yet), but please see the comment about the news.
news/new_parser_api.rst
Outdated
|
||
**Changed:** | ||
|
||
* import `loadData` and `deserialize_data` directly to integrate with `diffpy.utils(3.6.0)` |
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.
please move to fixed. this is a fix (bug) not a change (change in behavior of the program that users need to know about)
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.
Thank you. Updates have been pushed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #162 +/- ##
=======================================
Coverage 96.85% 96.85%
=======================================
Files 18 18
Lines 795 795
=======================================
Hits 770 770
Misses 25 25 |
@sbillinge It's ready for another review. |
Amazing! @ycexiao |
close #161