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

Fixed Issue #69 - Corrected typo in About Page #72

Conversation

Akhil-Jasson
Copy link
Contributor

  • Replaced the phrase "made need to be made" to "may need ..." in the 01_about page

- Replaced the phrase "made need to be made" to "may need ..."  in the 01_about page
@Akhil-Jasson
Copy link
Contributor Author

@aterrel @mikofski @tnabtaf @reshamas Could any of you please review my PR when you have a moment? Your feedback would be much appreciated. Thanks!

@Akhil-Jasson
Copy link
Contributor Author

Hi @reshamas @mikofski @aterrel , I’ve addressed the requested changes and ensured everything aligns with the project guidelines. Could you please review the updates when you get a chance? Let me know if further modifications are needed. Thanks!

@aterrel
Copy link
Member

aterrel commented Dec 22, 2024

@Akhil-Jasson please respond to my comment. I don't see why we need to add the oryx file.

@aterrel
Copy link
Member

aterrel commented Dec 22, 2024

@reshamas approved the typo but when I try to build with Python 3.9 as the patch uses in the yaml file, I get an error (see below). I recommend you remove the yml changes and the additionaly oryx file and then it can be merges easily.

--

Output Path: /Users/aterrel/Dev/numfocus/DISCOVER-Cookbook/DISCOVER/_build/html [0/532]
Running Sphinx v7.4.7
loading translations [en]... done

Extension error:
Could not import extension sphinx_tags (exception: No module named 'sphinx_tags')
Traceback (most recent call last):
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/sphinx/registry.py", line 453, in load_extension
mod = import_module(extname)
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/importlib/init.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "", line 1030, in _gcd_import
File "", line 1007, in _find_and_load
File "", line 984, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'sphinx_tags'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/jupyter_book/sphinx.py", line 114, in build_sphinx
app = Sphinx(
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/sphinx/application.py", line 253, in init
self.setup_extension(extension)
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/sphinx/application.py", line 429, in setup_extension
self.registry.load_extension(self, extname)
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/sphinx/registry.py", line 456, in load_extension
raise ExtensionError(__('Could not import extension %s') % extname,
sphinx.errors.ExtensionError: Could not import extension sphinx_tags (exception: No module named 'sphinx_tags')

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/Users/aterrel/Downloads/yes/envs/py39/bin/jupyter-book", line 10, in
sys.exit(main())
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/click/core.py", line 1161, in call
return self.main(*args, **kwargs)
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/click/core.py", line 1082, in main
rv = self.invoke(ctx)
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/click/core.py", line 1697, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/click/core.py", line 1443, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/click/core.py", line 788, in invoke
return __callback(*args, **kwargs)
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/jupyter_book/cli/main.py", line 317, in build
builder_specific_actions(
File "/Users/aterrel/Downloads/yes/envs/py39/lib/python3.9/site-packages/jupyter_book/cli/main.py", line 528, in builder_specific_actions
raise RuntimeError(_message_box(msg, color="red", doprint=False)) from result
RuntimeError:

There was an error in building your book. Look above for the cause.

===============================================================================

Copy link
Member

@aterrel aterrel left a comment

Choose a reason for hiding this comment

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

This PR is not necessary.

The typo has been corrected by another PR.
It adds an additional uncessary file (oryx).
And if we are going to update the Python version (which is probably a good idea) we should really updated it to 3.13 not 3.9 which is going to be EOL soon.

Copy link
Member

Choose a reason for hiding this comment

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

Is this a local thing? Seems not to be needed for the Repo.

@Akhil-Jasson
Copy link
Contributor Author

Hi @aterrel,

Thank you for your feedback! I understand now that the typo has already been addressed in another PR. I also see your point regarding the unnecessary addition of the Oryx file and the recommendation to update Python to version 3.13 instead of 3.9.

Given this, it seems that this PR is no longer required. I’ll go ahead and close it to avoid any confusion. Please let me know if there are any other areas where I can contribute—I’d love to help out!

@aterrel
Copy link
Member

aterrel commented Dec 23, 2024

Give a look to #74

Get Python 3.13 running, make sure the scripts and the instructions work and put it in one PR. Having PRs do more than one thing makes them harder to accept.

@Akhil-Jasson
Copy link
Contributor Author

Thank you, I sincerely appreciate your guidance. I’ll review #74 and proceed accordingly.

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.

3 participants