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

Fix typos in notebook #1967

Closed
wants to merge 2 commits into from
Closed

Fix typos in notebook #1967

wants to merge 2 commits into from

Conversation

giswqs
Copy link
Contributor

@giswqs giswqs commented Oct 26, 2023

Overview

This PR fixes some typos and formatting issues in the notebook.
https://docs.rastervision.io/en/0.21/usage/tutorials/reading_raster_data.html

image
image
image

Checklist

  • Added needs-backport label if PR is bug fix that applies to previous minor release
  • Ran scripts/format_code and committed any changes
  • Documentation updated if needed
  • PR has a name that won't get you publicly shamed for vagueness

Notes

Optional. Ancillary topics, caveats, alternative strategies that didn't work out, anything else.

Testing Instructions

  • How to test this PR
  • Prefer bulleted description
  • Start after checking out this branch
  • Include any setup required, such as rebuilding the Docker image.
  • Include test case, and expected output if not captured by automated tests.

Closes #XXX

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #1967 (2bce4fe) into master (7106652) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1967      +/-   ##
==========================================
- Coverage   86.19%   86.18%   -0.02%     
==========================================
  Files         191      191              
  Lines        9362     9362              
==========================================
- Hits         8070     8069       -1     
- Misses       1292     1293       +1     

see 1 file with indirect coverage changes

@@ -467,7 +467,7 @@
"tags": []
},
"source": [
"Another useful ``RasterTransformer`` is the :class:`.StatsTransformer`. \n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a ReST cell. ReST uses double back ticks for inline code.

@@ -159,7 +159,7 @@
"id": "83a6debf-e477-4b70-b82d-8e079ef8881e",
"metadata": {},
"source": [
"`RasterSource`s support `numpy`-like array slicing, so we can read a smaller chip within the full raster like so:"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The

`<class name>`s

convention is a deliberate choice. The trailing "s" is excluded from the inline code block to clarify that it is just for pluralization and not part of the class' name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is inconsistent. Some places uses s, some not.
https://docs.rastervision.io/en/0.21/usage/tutorials/reading_raster_data.html
image

image

Copy link
Collaborator

@AdeelH AdeelH Oct 27, 2023

Choose a reason for hiding this comment

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

That's true. The reason for that is a quirk of ReST/sphinx. The following is what I would like to do, but it does not work.

:class:`.RasterSource`s

So I ended up including the "s" whenever the class name is linked to the API docs.

@@ -727,7 +727,7 @@
"tags": []
},
"source": [
"``Point`` and ``LineString`` geometries are not directly useable if doing, say, semantic segmentation. The cells below show an example of converting road geometries (given in the form of ``LineString``s) into polygons using the :class:`.BufferTransformer`."
Copy link
Collaborator

Choose a reason for hiding this comment

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

See other comment about ReST.

@@ -343,7 +343,7 @@
"source": [
":class:`RasterSources <raster_source.raster_source.RasterSource>` accept a list of :class:`RasterTransformers <raster_transformer.raster_transformer.RasterTransformer>`, all of which are automatically applied (in the order specified) to each chip sampled from that :class:`.RasterSource`.\n",
"\n",
"Below we'll look at two such `RasterTransformer`s:\n",
"Below we'll look at two such `RasterTransformers`:\n",
Copy link
Collaborator

@AdeelH AdeelH Oct 27, 2023

Choose a reason for hiding this comment

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

This text is not currently correctly rendered because it is using single back ticks instead of double back ticks.

Suggested change
"Below we'll look at two such `RasterTransformers`:\n",
"Below we'll look at two such ``RasterTransformer``s:\n",

@giswqs
Copy link
Contributor Author

giswqs commented Oct 27, 2023

I can close this PR if the fixes are unintended. No worries. I just wanted to point out the inconsistent uses of class names and ticks. Some are not rendered properly on the website.

@giswqs
Copy link
Contributor Author

giswqs commented Oct 27, 2023

image

@giswqs
Copy link
Contributor Author

giswqs commented Oct 27, 2023

Never mind. I am closing it.

@giswqs giswqs closed this Oct 27, 2023
@AdeelH
Copy link
Collaborator

AdeelH commented Oct 27, 2023

Fair enough. Thanks for the PR anyway, Appreciate the effort.

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.

None yet

2 participants