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

Refactor README.md and include Approximate Equality as part of the RealModule. #150

Merged
merged 13 commits into from
Nov 9, 2020

Conversation

markuswntr
Copy link
Contributor

Approximate Equality has recently been merged into master. This PR updates the README.md to reflect the changes.
While it is part of the RealModule it really is an extension on Numeric, so I decided to list it below the RealModule but separately.

@8bitmp3
Copy link
Contributor

8bitmp3 commented Sep 2, 2020

Hey @markuswntr Thank for the PR. Big fan of Swift Numerics.

Take a look at markuswntr#1 - I've made a few improvement suggestions to the entire README based on tech writing experience - hope you like them and we can bundle the changes in this PR: #150:

Summary of suggestions:

  • Small change: "may migrate in the second" -> "may migrate into the second"
  • Refactor: "new uses are discovered" (passive voice) -> "more users start using Swift Numerics." (active voice)
  • Reduce the use of "simply" if possible (it's not necessary but is recommended)
  • Refactor + replace the idiom ("to pull in") for internalization/inclusiveness:
- Swift Numerics modules are fine-grained; if you need support for Complex
+ Swift Numerics modules are fine-grained. For example, if you need support for
+ Complex numbers ...
- ... import ComplexModule¹ without pulling in everything else in the library:
+ ... import ComplexModule¹ as a standalone module:
...
- as well.
...
[Example in code blocks]
  • Refactor:
- // All swift-numerics API is now available
+ // The entire `swift-numerics` API is now available
  • Replace the use of "we" with "the Swift Numerics project". For example:
- Because we intend to make it possible to adopt Swift Numerics modules in the
- standard library at some future point...
+ The Swift Numerics project is intended to be adopted in the standard library
+ in future. Therefore, it...
  • Provide three steps (1... 2... 3...) for the Using Swift Numerics in your project section, since it's a sequential process - this helps with UX
  • Small refactor: add "that is"
- Swift Numerics is a standalone library separate from the core Swift project. In
+ Swift Numerics is a standalone library that is separate from the core Swift project. In
  • Enhance several HowTo sections under "Contributing..." into discoverable subheadings (e.g. "to propose a new module" -> "# How to propose a new module")
  • Add a "### Forums" section for improved discoverability
  • Add "About" to enhance explanations of the links:
1. About [`RealModule`](Sources/RealModule/README.md)
    - [`ApproximateEquality`](Sources/RealModule/ApproximateEquality.swift)
2. About [`ComplexModule`](Sources/ComplexModule/README.md)
  • Reformat the Markdown text to wrap at 80 characters (it's a good practice, just like in code)
  • Turn complex sentences -> smaller easier to follow sentences
  • Use second person ("you") instead of first person ("I", "we"), if you can
  • Add blank lines between the section titles and first paragraphs
  • Add blank lines between before and after code blocks

Let me know what you think.

Cheers!

Enhance Swift Numerics README.md (technical writing perspective)
@markuswntr markuswntr changed the title Update README.md for Approximate Equality inclusion Update README.md Sep 2, 2020
@markuswntr markuswntr changed the title Update README.md Refactor README.md and include Approximate Equality as part of the RealModule. Sep 2, 2020
@markuswntr
Copy link
Contributor Author

Thanks to @8bitmp3, this PR now address not only the approximate equality module but also spelling/grammar - so I have changed the title to reflect the changes.

README.md Outdated
```swift
import Complex
// I know I only ever want Complex<Double>, so I shouldn't need the generic parameter.
typealias Complex = Complex.Complex<Double> // doesn't work, because name lookup fails.
// You only ever want Complex<Double>, so you shouldn't need the generic parameter.
Copy link
Member

@stephentyrone stephentyrone Sep 2, 2020

Choose a reason for hiding this comment

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

The comment here written by the hypothetical user of Swift Numerics (rather than us, the library authors), so "I" is appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we can use the second person ("you" - the reader/user) in docs instead of the first person ("I" and "we"), we should do so, according to Google and Microsoft style guides for doc writing. That's the reason for changing it here.

Copy link
Contributor

@xwu xwu Sep 2, 2020

Choose a reason for hiding this comment

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

I think @stephentyrone’s point here is that the comment here is not being written in the voice of the documentation writer, and what’s more essential, should not be expressed in that voice, since it is editorializing on particulars and not providing general advice.

Copy link
Contributor

Choose a reason for hiding this comment

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

@xwu @stephentyrone Gotcha! Agreed

README.md Outdated
```swift
import Numerics

// All swift-numerics API is now available
// The entire `swift-numerics` API is now available
Copy link
Member

Choose a reason for hiding this comment

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

swift-numerics doesn't seem to be appropriate formatting here; "swift-numerics" is not code. In prose we usually style the project "Swift Numerics".

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, thanks @stephentyrone

README.md Outdated
2. [ComplexModule](Sources/ComplexModule/README.md)

1. About [`RealModule`](Sources/RealModule/README.md)
- [`ApproximateEquality`](Sources/RealModule/ApproximateEquality.swift)
Copy link
Member

Choose a reason for hiding this comment

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

We should definitely link to detail on ApproximateEquality, but we should link to a markdown document, rather than the implementation. Let's remove this for now and open an issue to track adding one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I removed the link and opened an issue: #153

README.md Outdated
1. [RealModule](Sources/RealModule/README.md)
2. [ComplexModule](Sources/ComplexModule/README.md)

1. About [`RealModule`](Sources/RealModule/README.md)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think "About" adds any clarity here (it actually confuses it). This is a list of modules, and the module is not "About RealModule" it is "RealModule".

Copy link
Contributor

Choose a reason for hiding this comment

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

@stephentyrone Sounds good! Maybe we can have the title of the section named "# More on modules" or something along those lines? What each link is about/does can be quite important to increase clarity, better a11y, etc. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the heading is fine as-is. It is a list of modules, not a section “on modules.”

Copy link
Contributor

Choose a reason for hiding this comment

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

I am just trying to follow commonly accepted guidelines of tech writing for good docs.

The idea in our case is to help any user - whether they're new to Swift or Swift Numerics - to understand whether they should click on each link (this is part of the user experience/human-computer interface fields). So, in a way, this contributes to inclusiveness and accessibility, which are important to keep in mind when writing doc specs.

A few words with basic info can save readers a trip to an external site that may not want to go to (as per the Google style guide, as I couldn't find any relevant info on Microsoft or Apple's guides).

  • For instance, this shows it's a README and I'd probably click on it, since READMEs are mostly common knowledge:
[`RealModule` - README](Sources/RealModule/README.md)
[About `RealModule`](Sources/RealModule/README.md)
  • Compare this the original, where it's not too clear whether the user, who hasn't spent as much time with Swift Numerics as us, should click on the link. (Is it an API doc, a .swift file, a blog post, or an official Apple reference?)
[`RealModule`](Sources/RealModule/README.md)

Anyway, these are just suggestions. Maybe Apple's tech writers can help here.

Cheers!

Copy link
Contributor

@xwu xwu left a comment

Choose a reason for hiding this comment

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

Personally, I would prefer not adding hard line wraps to this document. It makes future editing much more finicky, and the diffs much less readable. Since it is not the prevailing style for this project to line-wrap Markdown files, I would suggest leaving that alone.

README.md Outdated
modules with more specialized dependencies (or dependencies that are not
available on all platforms supported by Swift) belong in a separate package.

The Swift Numerics project is intended to be adopted in the standard library
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not true in toto; in fact, it is contradicted on its face earlier in the document.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @xwu !

@markuswntr Can you take a look at this?

The diff:

- Because we intend to make it possible to adopt Swift Numerics modules in the
- standard library at some future point...
+ The Swift Numerics project is intended to be adopted in the standard library
+ in future. Therefore, it...

Copy link
Member

Choose a reason for hiding this comment

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

I'm actually the project owner, and @xwu is correct, the original text is correct, and this suggested change is not. There will surely be some components of Swift Numerics that never make their way into the standard library.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @markuswntr for making the changes and @stephentyrone and @xwu for the feedback!

README.md Outdated
However, there is also a top-level `Numerics` module that simply re-exports the complete public interface of swift-numerics:

There is also a top-level `Numerics` module that re-exports the complete public
interface of swift-numerics:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
interface of swift-numerics:
interface of Swift Numerics:

@8bitmp3
Copy link
Contributor

8bitmp3 commented Sep 3, 2020

I understand that line wraps aren't too user-friendly @xwu They are part of the docs-as-code approach and help users see "the end of the line", since Markdown rendering can't always auto-wrap depending on where you view the files. This linting style has been adopted by certain well-known open source Google projects (they even include it in doc tests).

@stephentyrone
Copy link
Member

stephentyrone commented Sep 3, 2020

They are part of the docs-as-code approach

I'm confused by this claim; as @xwu called out, wrapping at 80-cols makes diffs much larger than they should be for minor changes, which makes code review harder and more error-prone. This seems to be contrary to "docs-as-code" (one of the things code should permit is easily seeing how it has changed over time, as well as reviewing changes).

Markdown rendering can't always auto-wrap

Which renderers can't? Certainly whatever Github uses to render for the web does, and so does Xcode, which are by far the two renderers that will be most commonly used. Are there specific renderers that don't, which we expect to be frequently used with the Swift Numerics documentation (and are there reasons why viewing the docs on the web is not a viable option in those cases?)

@8bitmp3
Copy link
Contributor

8bitmp3 commented Sep 3, 2020

@stephentyrone I understand where you and @xwu come from and the GitHub web UI helps to easily the entire text, while applying soft-wrapping and other user-friendly features.

I can revert the wrapping changes back so the diffs are more visible - whatever makes it easier for you to review here 👍


Which renderers can't?

I've reviewed code and docs in VSCode and used various IDEs - from IDEA to Xcode - and I'm aware some folks don't always have the default settings set to "auto-wrap": "on". However, many things in modern IDEs are automated by default these days, including the awesome Xcode.

So, for instance, if a contributor forks a repo, opens a Markdown file, and decides to make certain changes, they may see some long lines that make up long paragraphs, such as:

image

For some background, there's a GitLab blog post about the pros and cons of wrapping text, that summarizes this quite well:

image

https://about.gitlab.com/blog/2016/10/11/wrapping-text/

Unwrap text, update Swift Numerics README
README.md Outdated

- API that is too specialized to go into the standard library, but which is sufficiently general to be centralized in a single common package.
- API that is under active development toward possible future inclusion in the standard library.

There is some overlap between these two categories, and API that begins in the first category may migrate to the second as it matures and new uses are discovered.
There is some overlap between these two categories, and an API that begins in the first category may migrate into the second as it matures and more users start using Swift Numerics.
Copy link
Contributor

Choose a reason for hiding this comment

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

Originally: "...and new uses are discovered." was 100% good. Let's revert here

README.md Outdated
Swift Numerics provides a set of modules that support numerical computing in Swift.
These modules fall broadly into two categories:

Swift Numerics provides a set of modules that support numerical computing in Swift. These modules fall broadly into two categories:
Copy link
Member

Choose a reason for hiding this comment

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

While we don't want to wrap, we do want one sentence per line.

Suggested change
Swift Numerics provides a set of modules that support numerical computing in Swift. These modules fall broadly into two categories:
Swift Numerics provides a set of modules that support numerical computing in Swift.
These modules fall broadly into two categories:

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. The proposed change looks good - will incorporate, thanks @stephentyrone

README.md Outdated
There is some overlap between these two categories, and API that begins in the first category may migrate to the second as it matures and new uses are discovered.
There is some overlap between these two categories, and an API that begins in the first category may migrate into the second as it matures and new uses are discovered.

Swift Numerics modules are fine-grained. For example, if you need support for Complex numbers, you can import ComplexModule¹ as a standalone module:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Swift Numerics modules are fine-grained. For example, if you need support for Complex numbers, you can import ComplexModule¹ as a standalone module:
Swift Numerics modules are fine-grained.
For example, if you need support for Complex numbers, you can import ComplexModule¹ as a standalone module:

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @stephentyrone - moving this sentence to the next paragraph LGTM

Copy link
Member

@stephentyrone stephentyrone Sep 3, 2020

Choose a reason for hiding this comment

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

To be clear, this is not a new paragraph. New paragraphs only occur in markdown on a double line break. Putting only one sentence per line falls out from docs-as-code practice; a sentence is a code statement.

Copy link
Contributor

Choose a reason for hiding this comment

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

@stephentyrone Got it!

README.md Outdated
The current modules assume only the availability of the Swift and C standard libraries and the runtime support provided by compiler-rt.
Future expansion may assume the availability of other standard interfaces such as [BLAS (Basic Linear Algebra Subprograms)](https://en.wikipedia.org/wiki/Basic_Linear_Algebra_Subprograms) and [LAPACK (Linear Algebra Package)](https://en.wikipedia.org/wiki/LAPACK), but modules with more specialized dependencies (or dependencies that are not available on all platforms supported by Swift) belong in a separate package.

Future expansion may assume the availability of other standard interfaces, such as [BLAS (Basic Linear Algebra Subprograms)](https://en.wikipedia.org/wiki/Basic_Linear_Algebra_Subprograms) and [LAPACK (Linear Algebra Package)](https://en.wikipedia.org/wiki/LAPACK). But modules with more specialized dependencies (or dependencies that are not available on all platforms supported by Swift) belong in a separate package.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Future expansion may assume the availability of other standard interfaces, such as [BLAS (Basic Linear Algebra Subprograms)](https://en.wikipedia.org/wiki/Basic_Linear_Algebra_Subprograms) and [LAPACK (Linear Algebra Package)](https://en.wikipedia.org/wiki/LAPACK). But modules with more specialized dependencies (or dependencies that are not available on all platforms supported by Swift) belong in a separate package.
Future expansion may assume the availability of other standard interfaces, such as [BLAS (Basic Linear Algebra Subprograms)](https://en.wikipedia.org/wiki/Basic_Linear_Algebra_Subprograms) and [LAPACK (Linear Algebra Package)](https://en.wikipedia.org/wiki/LAPACK).
But modules with more specialized dependencies (or dependencies that are not available on all platforms supported by Swift) belong in a separate package.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 @stephentyrone Will make this change

README.md Outdated
Swift Numerics is a standalone library separate from the core Swift project.
In practice, it will act as a staging ground for some APIs that may eventually be incorporated into the Swift Standard Library, and when that happens such changes will be proposed to the Swift Standard Library using the established evolution process of the Swift project.

Swift Numerics is a standalone library that is separate from the core Swift project. In practice, it will act as a staging ground for some APIs that may eventually be incorporated into the Swift Standard Library. When that happens, such changes will be proposed to the Swift Standard Library using the established evolution process of the Swift project.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Swift Numerics is a standalone library that is separate from the core Swift project. In practice, it will act as a staging ground for some APIs that may eventually be incorporated into the Swift Standard Library. When that happens, such changes will be proposed to the Swift Standard Library using the established evolution process of the Swift project.
Swift Numerics is a standalone library that is separate from the core Swift project, but it will sometimes act as a staging ground for APIs that will later be incorporated into the Swift Standard Library.
When that happens, such changes will be proposed to the Swift Standard Library using the established evolution process of the Swift project.

Copy link
Contributor

@8bitmp3 8bitmp3 Sep 3, 2020

Choose a reason for hiding this comment

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

Agree with the rewording and splitting into separate lines (not paragraphs) 👍 Will make these changes

README.md Outdated
¹ Swift is currently unable to use the fully-qualified name for types when a type and module have the same name (discussion here: https://forums.swift.org/t/pitch-fully-qualified-name-syntax/28482).
This would prevent users of Swift Numerics who don't need generic types from doing things like:

¹ Swift is currently unable to use the fully-qualified name for types when a type and module have the same name (discussion here: https://forums.swift.org/t/pitch-fully-qualified-name-syntax/28482). This would prevent users of Swift Numerics who don't need generic types from doing things, such as:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
¹ Swift is currently unable to use the fully-qualified name for types when a type and module have the same name (discussion here: https://forums.swift.org/t/pitch-fully-qualified-name-syntax/28482). This would prevent users of Swift Numerics who don't need generic types from doing things, such as:
¹ Swift is currently unable to use the fully-qualified name for types when a type and module have the same name (discussion here: https://forums.swift.org/t/pitch-fully-qualified-name-syntax/28482).
This would prevent users of Swift Numerics who don't need generic types from doing things, such as:

Copy link
Contributor

Choose a reason for hiding this comment

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

👍👍

README.md Outdated
New modules have to evaluate this decision carefully, but can err on the side of adding the suffix.
It's expected that most users will simply `import Numerics`, so this isn't an issue for them.

The `Real` module does not contain a `Real` type, but does contain a `Real` protocol, and users may want to define their own `Real` type (and possibly re-export the `Real` module) - that is why the suffix is also applied there. New modules have to evaluate this decision carefully, but can err on the side of adding the suffix. It's expected that most users will simply `import Numerics`, so this isn't an issue for them.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `Real` module does not contain a `Real` type, but does contain a `Real` protocol, and users may want to define their own `Real` type (and possibly re-export the `Real` module) - that is why the suffix is also applied there. New modules have to evaluate this decision carefully, but can err on the side of adding the suffix. It's expected that most users will simply `import Numerics`, so this isn't an issue for them.
The `Real` module does not contain a `Real` type, but does contain a `Real` protocol.
Users may want to define their own `Real` type (and possibly re-export the `Real` module)--that is why the suffix is also applied there.
New modules have to evaluate this decision carefully, but can err on the side of adding the suffix.
It's expected that most users will simply `import Numerics`, so this isn't an issue for them.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍👍 Thanks @stephentyrone

Update Swift Numerics README to address feedback
README.md Outdated
¹ Swift is currently unable to use the fully-qualified name for types when a type and module have the same name (discussion here: https://forums.swift.org/t/pitch-fully-qualified-name-syntax/28482).
This would prevent users of Swift Numerics who don't need generic types from doing things like:
This would prevent users of Swift Numerics who don't need generic types from doing things, such as:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This would prevent users of Swift Numerics who don't need generic types from doing things, such as:
This would prevent users of Swift Numerics who don't need generic types from doing things such as:

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it @xwu Thanks!

README.md Outdated
Future expansion may assume the availability of other standard interfaces such as [BLAS (Basic Linear Algebra Subprograms)](https://en.wikipedia.org/wiki/Basic_Linear_Algebra_Subprograms) and [LAPACK (Linear Algebra Package)](https://en.wikipedia.org/wiki/LAPACK), but modules with more specialized dependencies (or dependencies that are not available on all platforms supported by Swift) belong in a separate package.

Future expansion may assume the availability of other standard interfaces, such as [BLAS (Basic Linear Algebra Subprograms)](https://en.wikipedia.org/wiki/Basic_Linear_Algebra_Subprograms) and [LAPACK (Linear Algebra Package)](https://en.wikipedia.org/wiki/LAPACK).
But modules with more specialized dependencies (or dependencies that are not available on all platforms supported by Swift) belong in a separate package.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
But modules with more specialized dependencies (or dependencies that are not available on all platforms supported by Swift) belong in a separate package.
, but modules with more specialized dependencies (or dependencies that are not available on all platforms supported by Swift) belong in a separate package.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for reviewing @xwu, appreciate it. If you think it's important to have this section as one long sentence, I can definitely make this change!

I noticed that in GitHub's web UI, the whole paragraph takes up three lines. If we merge these sentences, they become a one long thing spanning three lines:

image


Here's the reasoning behind simplifying a large sentence into two shorter ones (these are guidelines, not rules):

From Google Technical Writing - Short sentences:

"Shorter documentation reads faster than longer documentation."

"Finding the shortest documentation implementation takes time but is ultimately worthwhile. Short sentences communicate more powerfully than long sentences, and short sentences are usually easier to understand than long sentences."

From Microsoft Style Guide - Writing Tips:

"These practices will help localizers and customers.
"Write short, simple sentences. Punctuating a sentence with more than a few commas and end punctuation usually indicates a complex sentence. Consider rewriting it or breaking it into multiple sentences."

https://developers.google.com/tech-writing/one/short-sentences

Copy link
Contributor

Choose a reason for hiding this comment

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

This is one thought, not two, so it needs to be one sentence.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok @xwu! Will make the changes

Update Swift Numerics README to address feedback
@stephentyrone
Copy link
Member

Hi @markuswntr, can you update this to point against main instead of master? Thanks.

@markuswntr markuswntr changed the base branch from master to main October 21, 2020 18:54
@markuswntr
Copy link
Contributor Author

Hi @stephentyrone, updated base branch to main. Thanks!

@stephentyrone stephentyrone merged commit e042c13 into apple:main Nov 9, 2020
@markuswntr markuswntr deleted the master branch November 17, 2020 12:30
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.

4 participants