Skip to content

Conversation

henryiii
Copy link
Contributor

@henryiii henryiii commented Jan 29, 2021

Adding three things (one commit each):

  • Support for [docs] and [test] extras, allowing these to be installed individually. Dev no longer includes the docs requirements (but the requirements-dev file does both for you, just like before). Simplifies RtD build a little bit. Bumped the pinned versions of docs requirements. Edit: [test] not [tests], more common (checked Pandas).
  • Bump to showing the 20.04 images, since the default should be changing soon. (GHA only, not sure about Azure or others)
  • Support nested markdown in other constructs. @joerick I think there might be one other place where this was needed? Removed for now, might be ruining styling

Copy link
Contributor

@joerick joerick left a comment

Choose a reason for hiding this comment

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

Something seems to have gone wrong with our styling, here...

Before After
home-before home-after
setup-before setup-after
option-before option-after

@@ -1 +1 @@
-e .[dev]
-e .[dev,docs]
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a little odd - shouldn't requirements-dev match [dev]? and [dev] include the requirements for building docs? Could the [dev] extra depend on other extras?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extras can't depend on other extras (my biggest pet peeve - and one that Poetry seems to not have solved when they built a more complicated requirements system), so they'd have to be listed twice. However, this means there might something missing from docs. I thought it was a little nicer to split "docs" from "dev", as often developers are not rebuilding the docs, so it gives more freedom there (since it's easy to ask for multiple extras. You can't as easily make multiple requirements files (it's ugly), and for historical purposes, the requirements-dev should stay the same. But I can also list them twice if better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: you can move extras back to setup.py, then you can make them depend on either other via code, I do this for extras sometimes: https://github.com/scikit-hep/boost-histogram/blob/918caebdf922097fde92df5a5de5442d86aa04c7/setup.py#L51-L64

I often have an "all" if I do this this way.

@henryiii
Copy link
Contributor Author

That is really weird, I don't see what would cause such a change in the styling... Wait, I wonder if it's super fences? It's now able to "nest" syntax, so... Still don't see how that would change un-fenced regular styling, like the table.

@joerick
Copy link
Contributor

joerick commented Jan 30, 2021

it might be the mkdocs update, maybe they changed the theme?

@henryiii
Copy link
Contributor Author

Maybe it could be the newer versions? If we have customizations depending on the version of these dependencies, perhaps? Later I'll try to bisect it and see which change causes the failure.

@henryiii henryiii marked this pull request as draft January 30, 2021 19:35
@henryiii
Copy link
Contributor Author

henryiii commented Feb 1, 2021

I think it was the super fences causing different HTML produced for nested constructs. No, some of the theming issues are still there. The one I was looking at (header-like thing above examples) was affected by the fences, but some of the other spacing issues are still there. Maybe things got renamed, breaking extras.css.

My real goal was to try to set up a way to add tabs (which I didn't manage yet / here, this was just some cleanup while playing with ideas). I think having tabs could really help in a few places:

The tab support for mkdocs seems to be for other themes, so we'd probably have to roll our own.

@henryiii
Copy link
Contributor Author

henryiii commented Feb 1, 2021

@YannickJadoul
Copy link
Member

Should we change https://cibuildwheel.readthedocs.io/en/stable/options/#examples_4 to use https://github.com/scipy/oldest-supported-numpy ?

I guess so, if it's good practice, even though that would show off less the possibilities of pyproject.toml

@henryiii
Copy link
Contributor Author

henryiii commented Feb 1, 2021

I would use:

    "numpy==1.13.3; python_version<'3.5'",
    "oldest-supported-numpy; python_version>='3.5'",

This would still show off the abilities of toml, but it would not have to be updated on every Python release, and it includes things like PyPy too!

@YannickJadoul
Copy link
Member

That would be perfect, indeed, I'd say :-)

@henryiii
Copy link
Contributor Author

henryiii commented Feb 3, 2021

Okay, pushed just the cleanups; the version updates can go in later, not mixed into this PR, since they are affecting the style and will need more work. Superfences too, maybe. I'd like to work on adding tabs to the above mentioned places as well eventually.

@henryiii henryiii marked this pull request as ready for review February 3, 2021 03:19
Copy link
Contributor

@joerick joerick left a comment

Choose a reason for hiding this comment

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

Just the one note on pytest dev requirement below, then I'm happy

@henryiii
Copy link
Contributor Author

henryiii commented Feb 4, 2021

If you'd rather, I could rename all -> dev, so that requirements-dev.txt would match [dev], then the current "dev" could be "scripts" or "bin" or something like that to indicate it's the requirements for the bin directory. I think the current (in this PR) naming is more common, where you have a dev for developer requirements, a docs for building docs, and test which is just the part of dev that builds tests. Developers who want to build docs can do [dev,docs].

@henryiii henryiii mentioned this pull request Feb 4, 2021
@henryiii
Copy link
Contributor Author

henryiii commented Feb 5, 2021

We can tweak or iterate later, it's approved and I fixed the issue mentioned (though, technically, pytest is still there, it just is mismatched (one has a >4, one doesn't), so I plan to merge soonish. I've now started alphabetically sorting the includes, just like in requirements.txt, so that these don't get added multiple times or in different places.

@henryiii henryiii merged commit 99acc0a into pypa:master Feb 5, 2021
@henryiii henryiii deleted the docs/cleanup branch February 5, 2021 03:02
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