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: syntax for strings python>=3.12 #439

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

FIX: syntax for strings python>=3.12 #439

wants to merge 6 commits into from

Conversation

mmcky
Copy link
Contributor

@mmcky mmcky commented Jan 11, 2025

This PR fixes a syntax warning across the lecture series

@HumphreyYang if you have time would you mind review this PR to make sure an r shouldn't be an rf for example. It would be nice to have a double check of it.

@HumphreyYang
Copy link
Collaborator

Many thanks @mmcky. I will scan through the lectures.

@HumphreyYang
Copy link
Collaborator

Hi Matt,

I just pushed some other r cases I found in the lectures.

I think for this warning we need to look for any string that has a \ that is not escaped : (

@@ -1179,26 +1179,26 @@ fig, axs = plt.subplots(3, 3, figsize=(14, 10))
# quantities
for i, name in enumerate(['K', 'Y', 'Cy', 'Co']):
ax = axs[i//3, i%3]
ax.plot(range(T+1), quant_seq3[:T+1, i], label=name+', $\delta$s=0')
ax.plot(range(T+1), quant_seq4[:T+1, i], label=name+', $\delta$s=0.005')
ax.plot(range(T+1), quant_seq3[:T+1, i], label=rf'{name}, $\delta$s=0')
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 @HumphreyYang good catch.

@mmcky
Copy link
Contributor Author

mmcky commented Jan 12, 2025

thanks @HumphreyYang I hadn't searched for the embedded case. Nice catch.

@jstac
Copy link
Contributor

jstac commented Jan 12, 2025

Thanks @mmcky and @HumphreyYang for the fixes.

Once you get this working please roll out to the other lecture series.

@mmcky
Copy link
Contributor Author

mmcky commented Jan 13, 2025

@HumphreyYang once a r string is setup we need to remove any \\ and replace with \ so there are quite a few cases to think about :-).

Copy link

github-actions bot commented Jan 13, 2025

@github-actions github-actions bot temporarily deployed to pull request January 13, 2025 00:14 Inactive
@github-actions github-actions bot temporarily deployed to pull request January 13, 2025 00:17 Inactive
@mmcky
Copy link
Contributor Author

mmcky commented Jan 13, 2025

@HumphreyYang on my local I am getting Syntax Warnings for docstrings. Is this happening to you?

For example from lake_model this line

Finds the steady state of the system :math:`x_{t+1} = \hat A x_{t}`

in a docstring is causing issues with \hat.

It doesn't see to be an issue on the gh build.

@github-actions github-actions bot temporarily deployed to pull request January 13, 2025 01:31 Inactive
@HumphreyYang
Copy link
Collaborator

Thanks @mmcky,

I did not get this error message. This is interesting, perhaps we need to escape things in the docstring?

@mmcky
Copy link
Contributor Author

mmcky commented Jan 13, 2025

Thanks @mmcky,

I did not get this error message. This is interesting, perhaps we need to escape things in the docstring?

thanks for letting me know @HumphreyYang. It's an odd error. It shouldn't be causing issues as docstrings should be able to contain latex markup. I'll have to do some further digging to see if I can replicate this in another environment.

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