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

Update comparisons #107

Merged
merged 4 commits into from
May 3, 2019
Merged

Update comparisons #107

merged 4 commits into from
May 3, 2019

Conversation

aaronspring
Copy link
Collaborator

@aaronspring aaronspring commented May 3, 2019

Description

  • added unittests for all PM comparison (will be useful for later refactoring or speedup)
  • changed the way _m2e is calculated, now leaves out the reference member from ensemble mean

Type of change

Please delete options that are not relevant.

  • Bug fix: before didnt leave out the reference member from ensemble mean

How Has This Been Tested?

  • unittests

Checklist (while developing):

  • I have added docstrings to all new functions.
  • I have commented my code, particularly in hard-to-understand areas
  • Tests added for pytest, if necessary.

Pre-Merge Checklist (final steps):

  • I have rebased onto master or develop (wherever I am merging) and dealt with any conflictions.
  • I have squashed commits to a reasonable amount, and force-pushed the squashed commits.
  • All notebooks run with this branch installed. (please post a screenshot of treon compiling successfully)
  • pytest runs without breaking.

Discussion

Add any points to discuss in the PR thread. Issues with the code? Suggestions needed? Philosophy changes to the package?

References

Please add any references to manuscripts, textbooks, etc.

@aaronspring aaronspring self-assigned this May 3, 2019
@aaronspring aaronspring requested a review from bradyrx May 3, 2019 16:27
@bradyrx
Copy link
Collaborator

bradyrx commented May 3, 2019

@aaronspring, Since these are minor edits and all builds pass (thanks Travis for testing treon and pytest!) this should be good. Nothing sticks out as being problematic.

However, this is a good opportunity for you to practice rebasing from command line since there shouldn't be any conflictions. It's helpful to clean up commits before merging so the messages are more clear, etc.

My workflow:

git checkout master
git pull master --rebase
git checkout update_comparisons
git rebase master --interactive

From here you shouldn't have to deal with any conflictions, but can squash your commits. You'll see the guide on the bottom of the vim screen, but this could be squashed into something like two commits for instance. You just modify the "pick" before the commit to what you want.

  • "(e)dit" if you want a clearer commit message
  • "(s)quash" if you want to just push that commit message into the last one
  • "(f)ixup" to get rid of the message completely
  • "(p)ick" to keep the commit as is.

What took me forever to figure out is that once you finish this process, you have to do git push -f origin update_comparisons to force-push the updated commits here so they are reflected.

I'll merge once that's done!

@bradyrx
Copy link
Collaborator

bradyrx commented May 3, 2019

Just tag me here when that's done so I get the email.

@aaronspring
Copy link
Collaborator Author

I touched notebooks, then deleted them, ... next time.

cant I just use the green merge pull request bottom and squash there with the dropdown?

@bradyrx
Copy link
Collaborator

bradyrx commented May 3, 2019

Totally, if there's a squash option there. Didn't realize you could edit commits tehre.

@bradyrx
Copy link
Collaborator

bradyrx commented May 3, 2019

Ah I see now the "squash and merge" option.

@aaronspring aaronspring merged commit 6af09b0 into master May 3, 2019
@bradyrx
Copy link
Collaborator

bradyrx commented May 3, 2019

@aaronspring, thanks! I'm excited to talk about the "v1 release" goals and project board next week.

@bradyrx bradyrx deleted the update_comparisons branch May 3, 2019 17:23
bradyrx pushed a commit that referenced this pull request May 18, 2019
* sshfs annoying tmp files

* create unitests for all comparisons. change _m2e to remove ref from ens

*  reduced 3d area to increase speed



Former-commit-id: 6af09b0
ahuang11 pushed a commit that referenced this pull request May 22, 2019
* sshfs annoying tmp files

* create unitests for all comparisons. change _m2e to remove ref from ens

*  reduced 3d area to increase speed



Former-commit-id: 6af09b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants