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

add twine check to azure CI + 3.8 matrix #1602

Merged
merged 12 commits into from
Jan 25, 2021
Merged

add twine check to azure CI + 3.8 matrix #1602

merged 12 commits into from
Jan 25, 2021

Conversation

Zethson
Copy link
Member

@Zethson Zethson commented Jan 20, 2021

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 [email protected]

@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
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

@Zethson
Copy link
Member Author

Zethson commented Jan 20, 2021

@ivirshup perfectly fine with me.
Removed 3.6

So we are now testing against 3.7 and 3.8.

@Zethson
Copy link
Member Author

Zethson commented Jan 20, 2021

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

@ivirshup
Copy link
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
Copy link
Member

ivirshup commented Jan 20, 2021

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

@Zethson
Copy link
Member Author

Zethson commented Jan 20, 2021

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
Copy link
Member Author

Zethson commented Jan 20, 2021

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
Member

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

@Zethson
Copy link
Member Author

Zethson commented Jan 20, 2021

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
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

@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
Copy link
Member

@ivirshup ivirshup left a comment

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.

.azure-pipelines.yml Outdated Show resolved Hide resolved
.azure-pipelines.yml Outdated Show resolved Hide resolved
@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
Member

@meeseeksdev backport to 1.7.x

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