Skip to content

Fixes binomial coefficients notation as of #83 #113

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
Merged
merged 3 commits into from
Nov 4, 2016
Merged

Fixes binomial coefficients notation as of #83 #113

merged 3 commits into from
Nov 4, 2016

Conversation

gabrielsimoes
Copy link
Contributor

Also added a note to contrib.md. Closes #83

tcNickolas
tcNickolas reviewed Oct 17, 2016
View reviewed changes
src/algebra/all-submasks.md

$$\sum_{k=0}^n C_n^k 2^k$$
$$\sum_{k=0}^n {n \choose k 2^k}$$
Copy link
Member

Choose a reason for hiding this comment

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

Please fix this formula, 2^k is supposed to be outside of binomial coefficient

@tcNickolas
Copy link
Member

Is there a specific reason to use both \binom and \choose? I'd rather stick with one format across all articles, say, \binom, as suggested in the original issue (and we're already using \binom somewhere I think).

tcNickolas
tcNickolas reviewed Oct 17, 2016
View reviewed changes
src/contrib.md
@@ -44,6 +44,10 @@ And here is the formula in the separate block:

$$E = mc^{2}$$

###Some conventions

* We have agreed as of issue [#83](https://github.com/e-maxx-eng/e-maxx-eng/issues/83) to express binomial coefficients with `\binom{n}{k}` or `{n \choose k}` instead of `C_n^k`.
Copy link
Member

Choose a reason for hiding this comment

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

I'd add matching formulas to different expressions, so that it's evident that the difference is in the result of the rendering, not just in formula syntax.

tcNickolas
tcNickolas requested changes Oct 17, 2016
View reviewed changes
Copy link
Member

@tcNickolas tcNickolas left a comment

Choose a reason for hiding this comment

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

I'll do a more thorough review later

@gabrielsimoes
Copy link
Contributor Author

gabrielsimoes commented Oct 17, 2016 •
edited
Loading

As far as I know, there is no major difference between them two (maybe when writting complex formulas inside the binomial, which probably won't happen). I'll stick with \binom{}{} as it is closer to other math symbols such as \frac.

EDIT: I'll adress the requests soon.

@gabrielsimoes
Copy link
Contributor Author

I changed all the formulas to \binom{}{} and added the rendered formulas to contrib.md as requested.

@tcNickolas
Copy link
Member

Sorry for the delay in reviewing. The change looks good to me, but now it has accumulated some conflicts. Could you please resolve them?

@gabrielsimoes
Copy link
Contributor Author

No problem, conflicts resolved.

tcNickolas
tcNickolas approved these changes Nov 4, 2016
View reviewed changes
Copy link
Member

@tcNickolas tcNickolas left a comment

Choose a reason for hiding this comment

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

Great, thank you!

@tcNickolas tcNickolas merged commit 5023769 into cp-algorithms:master Nov 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcNickolas tcNickolas tcNickolas 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.
2 participants
@gabrielsimoes @tcNickolas