-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Replace mdsynthesis with datreant #110
Conversation
e6fcf5b
to
859de85
Compare
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.
Looks good. Only some minor things
setup.py
Outdated
@@ -71,7 +71,7 @@ def get_property(prop, project): | |||
packages=find_packages(), | |||
install_requires=[ | |||
"numpy>=1.8", | |||
"mdsynthesis==0.6.1", | |||
"datreant==1.0.2", |
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.
datreant>=1.0.2
the API is stable. There are no changes planned.
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 we should pin versions none the less, as this will guarantee functionality for our users.
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 give the dependency resolver better resolution chances if you use minimal restrictions. This especially a concern with pip who doesn't do version resolution at all during updates.
.travis.yml
Outdated
@@ -15,7 +15,7 @@ env: | |||
global: | |||
- MAIN_CMD="pytest" | |||
- SETUP_CMD="mdbenchmark -v" | |||
- CONDA_DEPENDENCIES="mdsynthesis jinja2 click pandas matplotlib xdg<2" | |||
- CONDA_DEPENDENCIES="datreant jinja2 click pandas matplotlib xdg<2" |
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 should version the click dependency here as you do in the setup.py. Otherwise, you could get unexpected errors.
9db454e
to
d92c00d
Compare
Codecov Report
@@ Coverage Diff @@
## develop #110 +/- ##
===========================================
+ Coverage 96.31% 96.34% +0.03%
===========================================
Files 13 14 +1
Lines 651 684 +33
===========================================
+ Hits 627 659 +32
- Misses 24 25 +1
Continue to review full report at Codecov.
|
Fixes #99.
Changes made in this pull request:
mdsynthesis
withdatreant
.XX.[...].json
format to new.datreant/
folder.PR Checklist
./changelog/
(more information)?