Skip to content

Numpy Section Rewrite#346

Merged
tbalson merged 8 commits intocloudmesh-community:masterfrom
anthonyduer:patch-5
Feb 16, 2019
Merged

Numpy Section Rewrite#346
tbalson merged 8 commits intocloudmesh-community:masterfrom
anthonyduer:patch-5

Conversation

@anthonyduer
Copy link
Copy Markdown
Contributor

Rewrote the section on NumPy to make it more comprehensive and thorough. Included several new examples of code and discussed datatypes and two new reference. :)

Copy link
Copy Markdown
Contributor

@laszewsk laszewsk left a comment

Choose a reason for hiding this comment

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

we do not use the word above and below, but previous and next

Copy link
Copy Markdown
Contributor

@laszewsk laszewsk left a comment

Choose a reason for hiding this comment

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

use ## for subsections

use

a=1

for python scripts

make double sure there is no plagiarizm in examples

I used 'a' for arrays and 'm' for matrices to avoid confusion when switching among the topics. All example code was done extemporaneously as I rewrote the section. However, I did include a link to Wikipedia's Mean Squared Error page as a reference.
Copy link
Copy Markdown
Contributor

@laszewsk laszewsk left a comment

Choose a reason for hiding this comment

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

TA needs to review. I think some function descriptions from the original text were removed such as eig and diag ...

Tyler woudl be a good choice. Work with the TA

@laszewsk laszewsk added the help wanted Extra attention is needed label Feb 7, 2019
@laszewsk laszewsk added this to the sections milestone Feb 7, 2019
@anthonyduer
Copy link
Copy Markdown
Contributor Author

@tbalson Hey Tyler - would you mind reviewing my NumPy rewrite? I removed the eig() and diag() functions from the Linear Algebra section but can add them back in, if necessary. I had assumed they were a bit specific for a short section.

@laszewsk
Copy link
Copy Markdown
Contributor

laszewsk commented Feb 9, 2019

you need to add all deleted examples if they are not improved by other examples you did.
Also its unclear if now the examples are plagerized by you, so you need to verify this with the original source, and if they are create new examples

Restored all of the examples from the first version.
@anthonyduer
Copy link
Copy Markdown
Contributor Author

I've added all of the examples I removed back into the section - sorry about that!

For the new examples, obviously I don't want to plagiarize, but I'm not certain how to prove it - I generated the examples in Python by myself using NumPy using fairly nondescript sequences of numbers (1, 2, 3...) for the most part.

@laszewsk
Copy link
Copy Markdown
Contributor

tyler please review and let me know if we can/should approve

@laszewsk laszewsk modified the milestones: sections, 02-15-2019 Feb 12, 2019
Copy link
Copy Markdown
Contributor

@tbalson tbalson left a comment

Choose a reason for hiding this comment

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

Overall this contribution is well done however there are grammatical errors. I have highlighted several but not all at the end of this comment. I suggest that we commit (after another proof read) and I will create an issue for @anthonyduer and myself to proof read. For now @anthonyduer please proof read again and at least fix the errors I have highlighted.

The discussion about dimensions in NumPy basics is very image processing orientated, examples with N-dimensional data are limitless perhaps examples using something outside IP could be beneficial.

'For example, a uint8 can only contained' contained should be contain

'Finally, while NumPy will convenient convert between datatypes' convenient should be conveniently

@laszewsk
Copy link
Copy Markdown
Contributor

can you check that no examples were lost

Proofread the section and fixed several errors. I also changed some of the wording to be less awkward.
@anthonyduer
Copy link
Copy Markdown
Contributor Author

I've updated the grammar, mixed some spelling errors, and proofread the section again - sorry about the grammatical errors. I'll try and pull things into Word/Google Docs rather than editing directly in GitHub to avoid the issue in the future.

@tbalson
Copy link
Copy Markdown
Contributor

tbalson commented Feb 12, 2019

Original examples enhanced/retained.

@tbalson
Copy link
Copy Markdown
Contributor

tbalson commented Feb 12, 2019

We need to discuss relevant uses for eig and diag instead of just having them at the end. We can create an issue for this as it has enough substance be be another section...maybe on ML for IP with scikit learn.

@anthonyduer
Copy link
Copy Markdown
Contributor Author

We need to discuss relevant uses for eig and diag instead of just having them at the end. We can create an issue for this as it has enough substance be be another section...maybe on ML for IP with scikit learn.

Agreed - they were there in the original and I kept them at the professor's request but they felt a little out-of-place in the original version.

@laszewsk
Copy link
Copy Markdown
Contributor

eig and diag are good examples of funtions beyond trivial mean and so on.

Copy link
Copy Markdown
Contributor

@tbalson tbalson left a comment

Choose a reason for hiding this comment

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

No obvious plagiarism that I could ID, I will merge this however I request that you look in the book (our book) for more descriptions or uses for eig/diag if they already exist please reference them in this section, if there are none inform me and I will make a note of this.

@tbalson tbalson merged commit d58fb1f into cloudmesh-community:master Feb 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants