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

👌 IMPROVE: Fail gracefully on non-dict front-matter #409

Merged
merged 4 commits into from
Aug 6, 2021

Conversation

chrisjsewell
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Aug 6, 2021

Codecov Report

Merging #409 (674ab82) into master (5fd4c51) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #409      +/-   ##
==========================================
+ Coverage   90.65%   90.67%   +0.02%     
==========================================
  Files          14       14              
  Lines        1851     1855       +4     
==========================================
+ Hits         1678     1682       +4     
  Misses        173      173              
Flag Coverage Δ
pytests 90.67% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
myst_parser/docutils_renderer.py 92.73% <100.00%> (+0.01%) ⬆️
myst_parser/sphinx_renderer.py 93.08% <100.00%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5fd4c51...674ab82. Read the comment docs.

@chrisjsewell chrisjsewell merged commit 61f1154 into master Aug 6, 2021
@chrisjsewell chrisjsewell deleted the chrisjsewell-patch-1 branch August 6, 2021 03:50
@cpitclaudel
Copy link
Contributor

Is this patch actually safe? assert isn't guaranteed to run.

@chrisjsewell
Copy link
Member Author

Yeh that’s why it’s caught by the try/except

@cpitclaudel
Copy link
Contributor

No, I mean assert isn't guaranteed to run (I don't mean to succeed; I mean that Python can skip it entirely if you run MyST with -O). Maybe I'm misunderstanding the point of that test.

@chrisjsewell
Copy link
Member Author

run MyST with -O

can you explain further what you mean by this, and when this would be the case thanks?

@cpitclaudel
Copy link
Contributor

If you run Python with the -O flag, it skips all assertions. This is why AFAIK it's usually not recommended to put stateful code in assertions or to depend on assertions for control flow.

@chrisjsewell
Copy link
Member Author

chrisjsewell commented Sep 8, 2021

Thanks for the explanation BTW @cpitclaudel; I'd "heard" that you should not use assert, and usually try to follow that, but didn't actually know why lol.
Out of interest, is there some concrete use-case where one would want to use the -O flag?

@cpitclaudel
Copy link
Contributor

Not too many AFAIK ^^

@chrisjsewell
Copy link
Member Author

ok cheers

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.

2 participants