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

pip install jupytext[myst] should install myst-parser==0.8.x #590

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

mwouts
Copy link
Owner

@mwouts mwouts commented Aug 20, 2020

Quick fix for #589

@codecov
Copy link

codecov bot commented Aug 20, 2020

Codecov Report

Merging #590 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #590      +/-   ##
==========================================
+ Coverage   98.99%   99.02%   +0.02%     
==========================================
  Files          90       90              
  Lines        8699     8883     +184     
==========================================
+ Hits         8612     8796     +184     
  Misses         87       87              
Impacted Files Coverage Δ
tests/conftest.py 100.00% <0.00%> (ø)
tests/test_cli.py 100.00% <0.00%> (ø)
tests/test_pep8.py 100.00% <0.00%> (ø)
tests/test_black.py 100.00% <0.00%> (ø)
tests/test_isort.py 100.00% <0.00%> (ø)
tests/test_config.py 100.00% <0.00%> (ø)
tests/test_mirror.py 100.00% <0.00%> (ø)
tests/test_combine.py 100.00% <0.00%> (ø)
tests/test_compare.py 100.00% <0.00%> (ø)
tests/test_execute.py 100.00% <0.00%> (ø)
... and 26 more

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 213acee...5f292f2. Read the comment docs.

@mwouts mwouts force-pushed the myst_parser_0.8.x branch from 83f39ae to 5f292f2 Compare August 20, 2020 03:22
@mwouts mwouts merged commit cd54f97 into master Aug 20, 2020
@mwouts mwouts deleted the myst_parser_0.8.x branch August 20, 2020 03:50
@matthewfeickert
Copy link
Contributor

This isn't really relevant, but if you wanted to have kept the compatible version syntax ~= this

jupytext/setup.py

Lines 57 to 58 in aeb2cf6

extras_require={
"myst": ["myst-parser>=0.8; python_version >= '3.6'", "myst-parser<0.9"],

could have just been

     "myst": ["myst-parser~=0.8.0; python_version >= '3.6'"], 

as that bounds to v0.8.X releases.

@mwouts
Copy link
Owner Author

mwouts commented Aug 27, 2020

Thanks @matthewfeickert . Well, before my commit we had something very similar to what you propose:

"myst": ["myst-parser~=0.8; python_version >= '3.6'"],

However, I was quite surprise to see that it was not working so well. Not only LGTM was not understanding our requirement, but also, locally in my tests, pip install jupytext[myst] was installing myst-parser==0.12.x instead of 0.8.x! Is this something you can reproduce (or not)? Should we report this?

@matthewfeickert
Copy link
Contributor

matthewfeickert commented Aug 27, 2020

Well, before my commit we had something very similar to what you propose:

"myst": ["myst-parser~=0.8; python_version >= '3.6'"],

Is this something you can reproduce (or not)? Should we report this?

I think this is just a misunderstanding of the syntax:

myst-parser~=0.8

means restrict myst-parser to v0.X where X >= 8, where you wanted to restrict to the v0.8.X range so you need myst-parser~=0.8.0. This is covered (the best that I've found) in PEP 440's "Compatible release" section

For a given release identifier V.N, the compatible release clause is approximately equivalent to the pair of comparison clauses:

>= V.N, == V.*

As an example, in a fresh virtual environment

(myst-example) $ pip install -qq "myst-parser~=0.8"
(myst-example) $ pip list | grep myst
myst-parser                   0.12.5
(myst-example) $ pip install --upgrade -qq "myst-parser~=0.8.0"
(myst-example) $ pip list | grep myst
myst-parser                   0.8.2

My understanding is that the idea here is that you're wanting to establish a lower bound on a release that will provide the API you want at the level of the minor or patch release, operating under the assumption that if people are actually using semantic versioning then pinning ~=1.X will allow you to get new features in minor releases (along with bug fixes in patches) while keeping in the v1.0 API of a library while ~=1.2.X means that you need to limit yourself to the v1.2 release of the library but you don't mind getting further patches.

Though not all libraries actually do this, and you might need to treat minor releases as you would normally treat major (c.f. scikit-hep/pyhf#999 for an example).

Does that make sense? Or am I misunderstanding what the problem is?

@mwouts
Copy link
Owner Author

mwouts commented Aug 27, 2020

Awesome! Thanks for taking the time to explain this! I'll use ~=0.8.0 then.

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