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

[docs] [serve] removed line numbers and fixed file name summary_model.py #34617

Merged
merged 2 commits into from
Apr 21, 2023

Conversation

angelinalg
Copy link
Contributor

@angelinalg angelinalg commented Apr 20, 2023

Why are these changes needed?

Copy and paste button was including line numbers in 3 code examples, which is a bad user experience.
Fixed error with filename. The command line instructions said python model.py but it should be python summary_model.py.

This addresses two issues in GH issue 34481, but not all of them.

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Copy link
Contributor

@architkulkarni architkulkarni left a comment

Choose a reason for hiding this comment

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

Actually it looks like this removes the line numbers altogether from the snippet altogether, rather than just making it so they aren't copied when the button is clicked. We refer to the line numbers in the snippet later on, so I think we either need the line numbers to be displayed or we need to remove the references to line numbers.

@shrekris-anyscale if I remember correctly, you had figured out some way of displaying the line numbers without them being copied, right?

@shrekris-anyscale
Copy link
Contributor

shrekris-anyscale commented Apr 20, 2023

@shrekris-anyscale if I remember correctly, you had figured out some way of displaying the line numbers without them being copied, right?

I don't think I found a way to make the clipboard button stop copying line numbers. If you manually highlight the text, it doesn't copy the numbers.

@angelinalg
Copy link
Contributor Author

Great catch, @architkulkarni! Thank you.

@angelinalg
Copy link
Contributor Author

@maxpumperla, does the Sphinx or plugin upgrades give us the ability to show the line numbers without compromising the copy-paste button functionality?

@angelinalg angelinalg changed the title [docs] removed line numbers and fixed file name summary_model.py [docs] [serve] removed line numbers and fixed file name summary_model.py Apr 21, 2023
@edoakes edoakes merged commit 68db8e4 into ray-project:master Apr 21, 2023
jjyao pushed a commit that referenced this pull request Apr 21, 2023
….py (#34617)

Copy and paste button was including line numbers in 3 code examples, which is a bad user experience. 
Fixed error with filename. The command line instructions said `python model.py` but it should be `python summary_model.py`.

This addresses two issues in GH issue 34481, but not all of them.

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for https://docs.ray.io/en/master/.
    - [ ] I've added any new APIs to the API Reference. For example, if I added a 
           method in Tune, I've added it in `doc/source/tune/api/` under the 
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
….py (ray-project#34617)

Copy and paste button was including line numbers in 3 code examples, which is a bad user experience.
Fixed error with filename. The command line instructions said `python model.py` but it should be `python summary_model.py`.

This addresses two issues in GH issue 34481, but not all of them.

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for https://docs.ray.io/en/master/.
    - [ ] I've added any new APIs to the API Reference. For example, if I added a
           method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: elliottower <[email protected]>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
….py (ray-project#34617)

Copy and paste button was including line numbers in 3 code examples, which is a bad user experience.
Fixed error with filename. The command line instructions said `python model.py` but it should be `python summary_model.py`.

This addresses two issues in GH issue 34481, but not all of them.

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for https://docs.ray.io/en/master/.
    - [ ] I've added any new APIs to the API Reference. For example, if I added a
           method in Tune, I've added it in `doc/source/tune/api/` under the
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(

Signed-off-by: Jack He <[email protected]>
architkulkarni pushed a commit to architkulkarni/ray that referenced this pull request May 16, 2023
….py (ray-project#34617)

Copy and paste button was including line numbers in 3 code examples, which is a bad user experience. 
Fixed error with filename. The command line instructions said `python model.py` but it should be `python summary_model.py`.

This addresses two issues in GH issue 34481, but not all of them.

## Checks

- [ ] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR.
- [ ] I've run `scripts/format.sh` to lint the changes in this PR.
- [ ] I've included any doc changes needed for https://docs.ray.io/en/master/.
    - [ ] I've added any new APIs to the API Reference. For example, if I added a 
           method in Tune, I've added it in `doc/source/tune/api/` under the 
           corresponding `.rst` file.
- [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
- Testing Strategy
   - [ ] Unit tests
   - [ ] Release tests
   - [ ] This PR is not tested :(
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.

5 participants