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

updated documentation instances of neighbour to neighbor for consistency #4309

Closed
wants to merge 1 commit into from

Conversation

ekohilas
Copy link

I noticed that there were inconsistencies with the spelling of "neighbor" as "neighbour" when recently using your library, and I wanted to contribute back given it's hackoctoberfest and make some quick updates to the spelling for consistency, correcting the instances to the american versions (which is what I assume is the preffered in this repository)

These were all the instances of "neighbour" in the documentation according to my recursive grep search.

@ekohilas
Copy link
Author

Additionally, it would be appreciated if the "hacktoberfest-accepted" label could be added to the PR, if this repository is open to it :)

@rossbar
Copy link
Contributor

rossbar commented Nov 8, 2020

I'm -1 on style changes to the documentation. Networkx doesn't have a policy re: documentation style, and I'm -1 on adopting changes that imply one.

There are many places where the documentation can be improved by adding things that are missing or improving technical accuracy, but I'm going to close this to avoid spending cycles on wordings.

@ekohilas
Copy link
Author

Hi @rossbar,

I would be understanding, however I came across another issue today where this came up. Knowing British English, it can be quite frustrating when searching through documentation using your localisation (which is understandable). But I think it's worse if you search through documentation and you have two different sets of results.

For example, the searches with neighbour vs neighbor

If I was a new user of this repository and wasn't used to using american English, those first results would be deceiving, and would make me think that what I was looking for doesn't exist. And from the other perspective, the results would be incomplete (which I think is a more important issue, especially for something as abundant as neighbors)

@ekohilas
Copy link
Author

Additionally, could you please explain what you mean by spending cycles? Is there a cost to submitting pull requests? I couldn't find any mention of this in the contributing documentation.

@rossbar
Copy link
Contributor

rossbar commented Nov 18, 2020

Thanks for following up @ekohilas

You make a good point about the documentation search, that's something I hadn't considered. In a perfect world, the search functionality would be robust enough to handle such cases, but as it stands that's clearly not the case.

The point I had tried to convey was to generally discourage subjective changes to the documentation. Without the search results as a motivation, this struck me as a simple preference for one wording over the other. TBH this may have been an overreaction on my part due to the mention of hacktoberfest, which is notorious for causing a lot of maintenence burden :)

Additionally, could you please explain what you mean by spending cycles?

Sorry, not very clear wording on my part --- I was referring to reviews. In most cases, PRs require review & approval from at least two people, and reviews take time. I (seemingly unfairly) saw multiple PRs with "neighbour"->"neighbor" proposals and assumed they were the opening salvo of such proposals ("colour" -> "color"; "localise" -> "localize", etc.) and I wanted to head that off.

Anyways - your point about the search is a good one (maybe worth raising with sphinx?). Feel free to re-open (perhaps folding in the changes from #4308 in here as well).

@dschult dschult reopened this Nov 19, 2020
@jarrodmillman
Copy link
Member

@ekohilas Please fold in your changes from #4308 incorporating Dan's suggestions regarding using nbr for variable names.

@jarrodmillman jarrodmillman marked this pull request as draft November 26, 2020 18:45
Base automatically changed from master to main March 4, 2021 18:20
@MridulS
Copy link
Member

MridulS commented Dec 14, 2023

Made the changes in #7162, thanks for opening this @ekohilas !

@MridulS MridulS closed this Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants