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.

Sign up for GitHub

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

Jump to bottom

[MRG] Add CI builds for Python 3.11 #1659

Merged
merged 5 commits into from
Oct 4, 2022
Merged

[MRG] Add CI builds for Python 3.11 #1659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
77824f1
Add CI builds for Python 3.11
mrbean-bremen Jul 8, 2022
d310978
Add 2.4 to version table
mrbean-bremen Jul 8, 2022
aeb00f2
Remove support for Python 3.6
mrbean-bremen Sep 21, 2022
54c16c5
Use pypy based on Python 3.7
mrbean-bremen Sep 22, 2022
c32358c
Fix syntax for pypy version
mrbean-bremen Sep 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/merge-pytest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
python-version: ['3.6', '3.7', '3.8', '3.9', '3.10']
python-version: ['3.7', '3.8', '3.9', '3.10', '3.11-dev']
include:
- os: ubuntu-latest
python-version: '3.10'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/merge-typing.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: [ '3.6', '3.7', '3.8', '3.9', '3.10' ]
python-version: [ '3.7', '3.8', '3.9', '3.10', '3.11-dev' ]

steps:
- uses: actions/checkout@v2
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pr-pytest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest]
python-version: [pypy3]
python-version: [pypy-3.7]
steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
Expand All @@ -36,7 +36,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
python-version: ['3.6', '3.7', '3.8', '3.9']
python-version: ['3.7', '3.8', '3.9', '3.10']
include:
- os: 'ubuntu-latest'
pytest-args: --cov=pydicom --cov-append
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pr-type-lint-spell.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: [ '3.6', '3.10' ]
python-version: [ '3.7', '3.10' ]

steps:
- uses: actions/checkout@v2
Expand Down
42 changes: 22 additions & 20 deletions doc/faq/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,27 +55,29 @@ libraries are required.
What version of Python can I use?
---------------------------------

+-----------------+------------------+---------------------------+
| pydicom version | Release date | Python versions |
+=================+==================+===========================+
| 1.0 | March 2018 | 2.7, 3.4, 3.5, 3.6 |
+-----------------+------------------+---------------------------+
| 1.1 | June 2018 | 2.7, 3.4, 3.5, 3.6 |
+-----------------+------------------+---------------------------+
| 1.2 | October 2018 | 2.7, 3.4, 3.5, 3.6 |
+-----------------+------------------+---------------------------+
| 1.3 | July 2019 | 2.7, 3.4, 3.5, 3.6 |
+-----------------+------------------+---------------------------+
| 1.4 | January 2020 | 2.7, 3.5, 3.6, 3.7, 3.8 |
+-----------------+------------------+---------------------------+
| 2.0 | May 2020 | 3.5, 3.6, 3.7, 3.8 |
+-----------------+------------------+---------------------------+
| 2.1 | November 2020 | 3.6, 3.7, 3.8, 3.9 |
+-----------------+------------------+---------------------------+
| 2.2 | August 2021 | 3.6, 3.7, 3.8, 3.9 |
+-----------------+------------------+--------------------------+
| pydicom version | Release date | Python versions |
+=================+==================+==========================+
| 1.0 | March 2018 | 2.7, 3.4, 3.5, 3.6 |
+-----------------+------------------+--------------------------+
| 1.1 | June 2018 | 2.7, 3.4, 3.5, 3.6 |
+-----------------+------------------+--------------------------+
| 1.2 | October 2018 | 2.7, 3.4, 3.5, 3.6 |
+-----------------+------------------+--------------------------+
| 1.3 | July 2019 | 2.7, 3.4, 3.5, 3.6 |
+-----------------+------------------+--------------------------+
| 1.4 | January 2020 | 2.7, 3.5, 3.6, 3.7, 3.8 |
+-----------------+------------------+--------------------------+
| 2.0 | May 2020 | 3.5, 3.6, 3.7, 3.8 |
+-----------------+------------------+--------------------------+
| 2.1 | November 2020 | 3.6, 3.7, 3.8, 3.9 |
+-----------------+------------------+--------------------------+
| 2.2 | August 2021 | 3.6, 3.7, 3.8, 3.9 |
+-----------------+------------------+--------------------------+
| 2.3 | ~February 2022 | 3.6, 3.7, 3.8, 3.9, 3.10 |
+-----------------+------------------+--------------------------+
| 2.3 | March 2022 | 3.6, 3.7, 3.8, 3.9, 3.10 |
+-----------------+------------------+---------------------------+
| 2.4 | ~September 2022 | 3.7, 3.8, 3.9, 3.10, 3.11 |
+-----------------+------------------+---------------------------+
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed 3.6 here as it is EOL since last year, but I'm not sure if this is correct - did we have some plan here?

Copy link
Member

Choose a reason for hiding this comment

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

I don't recall if we had a plan, but it makes sense to drop it.


What about support for Python 2.7?
----------------------------------
Expand Down
5 changes: 5 additions & 0 deletions doc/release_notes/v2.4.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
Version 2.4.0
=================================

Changes
-------
* Removed support for Python 3.6 (EOL since December 2021)

Enhancements
------------
* Added attribute :attr:`~pydicom.valuerep.PersonName.alphabetic` (:pr:`1634`)
Expand All @@ -9,6 +13,7 @@ Enhancements
validated VRs (:issue:`1414`)
* CLI commands now accept *pydicom* charset test files and CLI help shows
Python Version (:pr:`1674`)
* Added support for Python 3.11 (:issue:`1658`)

Fixes
-----
Expand Down
5 changes: 4 additions & 1 deletion pydicom/tests/test_fileset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@

import os
import sys
from pathlib import Path
import shutil
from tempfile import TemporaryDirectory
Expand Down Expand Up @@ -1225,7 +1226,9 @@ def test_path(self, tdir):
"""Test setting the File-set's path."""
fs = FileSet()
assert fs.path is None
with pytest.raises(AttributeError, match=r"can't set attribute"):
msg = (r"can't set attribute" if sys.version_info < (3, 11)
else r"property 'path' of 'FileSet' object has no setter")
with pytest.raises(AttributeError, match=msg):
fs.path = tdir.name

# Test with str
Expand Down
6 changes: 3 additions & 3 deletions pydicom/valuerep.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,9 +774,9 @@ class TM(_DateTimeBase, datetime.time):
"""
_RE_TIME = re.compile(
r"(?P<h>^([01][0-9]|2[0-3]))"
r"((?P<m>([0-5][0-9]))?"
r"(?(5)(?P<s>([0-5][0-9]|60))?)"
r"(?(7)(\.(?P<ms>([0-9]{1,6})?))?))$"
r"((?P<m>([0-5][0-9]))"
r"((?P<s>([0-5][0-9]|60))"
r"(\.(?P<ms>([0-9]{1,6})?))?)?)?$"
Comment on lines +778 to +779
Copy link

@Waszker Waszker Oct 25, 2022 •
edited

Choose a reason for hiding this comment

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

@mrbean-bremen I'm sorry to bring this up but could you please elaborate why this pattern works?

Looking at it I think that it's not the same as it used to be, e.g. I think that in some cases it allows for <ms> group to be matched even if the <s> one did not match. Previously this <ms> group matching was dependent on the success of matching <s> group (via the ?(7)... syntax), whereas now it's not.

Also, I wonder whether this isn't a bug in Python re module? The previous regex is correctly recognized by some other regexp implementation, e.g. https://regex101.com/

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that in some cases it allows for group to be matched even if the <s> one did not match

I don't see this - as far as I understand, the group is an optional part of the expression above, while the <s> group is mandatory (in that scope, it is optional in the scope of the <m> group). Not sure if that was clear :) Anyway, if you can show me an example that would now incorrectly match, I stand corrected, of course.

Also, I wonder whether this isn't a bug in Python re module?

Yes, it is - but I think you filed it yourself? Anywhere, it will eventually be fixed, but it is not fixed in Python 3.11.0, so we cannot remove the fix yet. In case the fix (in pydicom) is incorrect, we have to change it of course, otherwise we can just leave it as is.

Copy link

Choose a reason for hiding this comment

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

I think that in some cases it allows for group to be matched even if the <s> one did not match

I don't see this - as far as I understand, the group is an optional part of the expression above, while the <s> group is mandatory (in that scope, it is optional in the scope of the <m> group). Not sure if that was clear :) Anyway, if you can show me an example that would now incorrectly match, I stand corrected, of course.

Ah you're right! Somehow I didn't notice the removal of the ? on the <s> group...

Also, I wonder whether this isn't a bug in Python re module?

Yes, it is - but I think you filed it yourself? Anywhere, it will eventually be fixed, but it is not fixed in Python 3.11.0, so we cannot remove the fix yet. In case the fix (in pydicom) is incorrect, we have to change it of course, otherwise we can just leave it as is.

Yes, I have filled in a bug report once I've confirmed that there is a problem with Python's re module, but that was after posting the question here. Sorry for making it look so confusing 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I have filled in a bug report once I've confirmed that there is a problem with Python's re module

Thanks for that, by the way, should have done this myself!

Sorry for making it look so confusing

It got me to re-check the fix, which is always a good thing. Anyway, I understand that leaving the fix in is ok, regardless of the Python re fix, so all is well.

)

def __new__( # type: ignore[misc]
Expand Down
4 changes: 2 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ def data_files_inventory():
"Intended Audience :: Science/Research",
"Development Status :: 5 - Production/Stable",
"Programming Language :: Python",
"Programming Language :: Python :: 3.6",
"Programming Language :: Python :: 3.7",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Operating System :: OS Independent",
"Topic :: Scientific/Engineering :: Medical Science Apps.",
"Topic :: Scientific/Engineering :: Physics",
Expand All @@ -63,7 +63,7 @@ def data_files_inventory():
},
include_package_data=True,
zip_safe=False,
python_requires='>=3.6.1',
python_requires='>=3.7',
install_requires=[],
extras_require={
"docs": [
Expand Down