Skip to content

add twine check to azure CI + 3.8 matrix#1602

Merged
ivirshup merged 12 commits into
masterfrom
feature/twine_check
Jan 25, 2021
Merged

add twine check to azure CI + 3.8 matrix#1602
ivirshup merged 12 commits into
masterfrom
feature/twine_check

Conversation

@Zethson

@Zethson Zethson commented Jan 20, 2021

Copy link
Copy Markdown
Member

Dear everyone,

This PR adds

  1. Python 3.8 to the build matrix. We were currently only testing vs 3.6 and 3.7. 3.10 is already coming up in April, so it is in my opinion time to use the more latest Python versions. If you feel like this adds too much to the Azure/CI bill, then I would suggest to remove 3.6.
  2. Solves Add build and twine check CI task #1585

Signed-off-by: Zethson lukas.heumos@posteo.net

Signed-off-by: Zethson <lukas.heumos@posteo.net>
Signed-off-by: Zethson <lukas.heumos@posteo.net>
@Zethson Zethson changed the title add twine check to azure CI + 3.8 and 3.9 matrix add twine check to azure CI + 3.8 matrix Jan 20, 2021
@ivirshup

Copy link
Copy Markdown
Member

👍 to twine check 👎 to the python versions

Ultimately, we do have a limited amount of CI, so I think it's important to be a bit cautious adding many jobs. Of the dependencies I'm worried about being an issue: generally not newer python versions. Minimum python versions are important for catching us using newer features.

I am up for swapping python 3.7 with 3.8. I don't think 3.9 is going to work for now. Last time I tried to use 3.9 (a month ago) numpy builds weren't working. I believe numba currently isn't working: numba/numba#6345

Higher priorities to me (roughly in order):

  • Test on windows
  • Test against lower bounds of our requirements
  • Test on Mac
  • Dev builds

Signed-off-by: Zethson <lukas.heumos@posteo.net>
@Zethson

Zethson commented Jan 20, 2021

Copy link
Copy Markdown
Member Author

@ivirshup perfectly fine with me.
Removed 3.6

So we are now testing against 3.7 and 3.8.

@Zethson

Zethson commented Jan 20, 2021

Copy link
Copy Markdown
Member Author

@ivirshup is there any reason why we are currently not additionally using Github Actions?

@ivirshup

Copy link
Copy Markdown
Member

Removed 3.6

We should keep 3.6 as long as we support it. It's easy to accidentally add features which only work with 3.7+ otherwise. I'd be happy to drop 3.6 once numpy does (and in general roughly follow NEP 29 as soon as the ecosystem does).

is there any reason why we are currently not additionally using Github Actions?

Depends on the task. Also depends on the definition of github actions I think? We aren't using any of their runners for testing because we'd like the ability to integrate with hosted resources on azure. Also, azure seemed like much more of a standard for numeric python packages at the time we chose it.

I'd be happy to have github actions for other things, like precommit. twine check could be another one, but I haven't looked in to how "artifact" type things are handled with github actions to know if we'd be able to recover the built objects.

We'd talked about using codecov too, which I'd like to add a check for. I'm not totally clear on the distinction between checks and actions yet.

@ivirshup

ivirshup commented Jan 20, 2021

Copy link
Copy Markdown
Member

btw, it'd be great if this got a sibling PR in anndata

@Zethson

Zethson commented Jan 20, 2021

Copy link
Copy Markdown
Member Author

We should keep 3.6 as long as we support it. It's easy to accidentally add features which only work with 3.7+ otherwise. I'd be happy to drop 3.6 once numpy does (and in general roughly follow NEP 29 as soon as the ecosystem does).

All right.
So do you want
3.6 + 3.7 + 3.8
Or
3.6 + 3.7
?

@Zethson

Zethson commented Jan 20, 2021

Copy link
Copy Markdown
Member Author

Regarding Github Actions. It is in my opinion the fastest option out there and jobs spin up really fast.

We could certainly move some of the fast and easy checks to Github Actions and leave the more heavy jobs for Azure.
But let's keep this for a discussion in another issue or Slack.

I'll add a sibling PR to Anndata as soon as we merged this one.

@ivirshup

Copy link
Copy Markdown
Member

I think 3.6 + 3.8 would be reasonable, covering the upper and lower bounds.

@Zethson

Zethson commented Jan 20, 2021

Copy link
Copy Markdown
Member Author

I think 3.6 + 3.8 would be reasonable, covering the upper and lower bounds.

Feels weird to me honestly, but I see what you're trying to do.
Is there no possibility that our Azure credits cover 3.6, 3.7 and 3.8?

If not, then I would be fine with 3.6 and 3.8

@ivirshup

Copy link
Copy Markdown
Member

10 simultaneous jobs is the supposed limit (though I'm not sure how much this is enforced). I think upper and lower bounds are generally a good way to go:

  • lower bounds make sure we don't add incompatible features
  • upper bounds give us deprecation warnings

Signed-off-by: Zethson <lukas.heumos@posteo.net>
@Zethson Zethson marked this pull request as ready for review January 20, 2021 13:31
@Zethson Zethson requested a review from ivirshup January 20, 2021 13:31

@ivirshup ivirshup left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a minor change, otherwise all good.

Comment thread .azure-pipelines.yml Outdated
Signed-off-by: Zethson <lukas.heumos@posteo.net>
Signed-off-by: Zethson <lukas.heumos@posteo.net>
Signed-off-by: Zethson <lukas.heumos@posteo.net>
Signed-off-by: Zethson <lukas.heumos@posteo.net>
Signed-off-by: Zethson <lukas.heumos@posteo.net>
Comment thread .azure-pipelines.yml Outdated
@ivirshup ivirshup linked an issue Jan 25, 2021 that may be closed by this pull request
@ivirshup ivirshup merged commit a64a1e4 into master Jan 25, 2021
@ivirshup

Copy link
Copy Markdown
Member

@meeseeksdev backport to 1.7.x

meeseeksmachine pushed a commit to meeseeksmachine/scanpy that referenced this pull request Jan 25, 2021
ivirshup pushed a commit that referenced this pull request Jan 25, 2021
Co-authored-by: Lukas Heumos <lukas.heumos@posteo.net>
@flying-sheep flying-sheep deleted the feature/twine_check branch January 27, 2021 09:59
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.

Add build and twine check CI task

3 participants