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

Incorrect check for NoneType #95

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/pythonpackage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
PYTHON_VERSION: ["3.7", "3.8", "3.9", "3.10"]
PYTHON_VERSION: ["3.8", "3.9", "3.10", "3.11", "3.12"]
steps:
- uses: actions/checkout@v2

Expand Down
2 changes: 1 addition & 1 deletion fgpyo/fasta/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
'AAAAAAAAAANNN'

"""

import textwrap
from pathlib import Path
from typing import TYPE_CHECKING
Expand All @@ -48,7 +49,6 @@ def samtools_dict(*args: Any) -> None:
def samtools_faidx(*args: Any) -> None:
pass


else:
from pysam import dict as samtools_dict
from pysam import faidx as samtools_faidx
Expand Down
10 changes: 7 additions & 3 deletions fgpyo/util/inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ def split_at_given_level(


def _get_parser(
cls: Type, type_: TypeAlias, parsers: Optional[Dict[type, Callable[[str], Any]]] = None
cls: Type,
type_: TypeAlias,
parsers: Optional[Dict[type, Callable[[str], Any]]] = None,
) -> partial:
"""Attempts to find a parser for a provided type.

Expand Down Expand Up @@ -226,7 +228,7 @@ def dict_parse(dict_string: str) -> Dict[Any, Any]:
return types.make_enum_parser(type_)
elif types.is_constructible_from_str(type_):
return functools.partial(type_)
elif isinstance(type_, type(type(None))):
elif type_ is type(None): # noqa: E721
Copy link
Member

Choose a reason for hiding this comment

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

can you please add a test?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jdidion following up on your question from Slack, I think this is the right approach.

as far as I'm aware, flake8 guards against type(x) == int and the like. We're doing equality checks against types above (e.g. if type_ == bool) and flake8 doesn't complain, I'm guessing because we don't call type() in the same line.

If you wanted to avoid the noqa, you could declare NoneType = type(None) above (or in fgpyo.types and import it), then replace this line with elif type_ is NoneType. But this feels like an edge case in flake8, so I think what you have is fine if you're fine with it

return functools.partial(types.none_parser)
elif types.get_origin_type(type_) is Union:
return types.make_union_parser(
Expand Down Expand Up @@ -255,7 +257,9 @@ def dict_parse(dict_string: str) -> Dict[Any, Any]:


def attr_from(
cls: Type, kwargs: Dict[str, str], parsers: Optional[Dict[type, Callable[[str], Any]]] = None
cls: Type,
kwargs: Dict[str, str],
parsers: Optional[Dict[type, Callable[[str], Any]]] = None,
) -> Any:
"""Builds an attr class from key-word arguments

Expand Down
Loading
Loading