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

Minor cleanups for v0.2.5 #31

Merged
merged 5 commits into from Feb 14, 2019
Merged

Minor cleanups for v0.2.5 #31

merged 5 commits into from Feb 14, 2019

Conversation

dgasmith
Copy link
Collaborator

iSorts and YAPFs the entire code base + enhancement of tests.

@dgasmith dgasmith added the Enhancement Project enhancement discussion label Feb 14, 2019
@codecov
Copy link

codecov bot commented Feb 14, 2019

Codecov Report

Merging #31 into master will increase coverage by 0.97%.
The diff coverage is 96.82%.

Copy link
Collaborator

@loriab loriab left a comment

Choose a reason for hiding this comment

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

That isort does a good job of matching what I do by hand (except for my sorting of std lib imports by length:-) ). Others are judgement calls. Overall, looks good.

@@ -1,4 +1,3 @@

Copy link
Collaborator

Choose a reason for hiding this comment

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

Better to put in a yapf disable around whole file than let a versioneer generates file be edited, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would say, yes. But since we do not regenerate these, I say just YAPF once and get on with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So long as doesn’t interfere with upgrading versioneer, sounds fine. (I don’t think it should, just the one complication that comes to mind)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think versioneer will even be upgraded again to be honest. I think that repository is dead and PyPA is creating a replacement. See here.

Enhancements
++++++++++++

- (:pr:`31`) Lints the code base preparign for a release and minor test improvements.
Copy link
Collaborator

Choose a reason for hiding this comment

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

preparing

@@ -2033,4 +2033,4 @@
'uncertainty': '0.000 0017 e-3'
}
}
}
} # yapf: disable
Copy link
Collaborator

Choose a reason for hiding this comment

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

2 sp before comment

data = water_molecule.dict()
data["geometry"] = list(range(8))
with pytest.raises(ValidationError):
Molecule(**data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

For these very generic errors, if there’s more specific info in the error msg, it’s sometimes handy to test the string contents too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, best to put it on a TODO.

@dgasmith dgasmith merged commit a8c93ba into MolSSI:master Feb 14, 2019
@dgasmith dgasmith deleted the tweaks2 branch October 18, 2019 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Project enhancement discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants