Git Product home page Git Product logo

Comments (7)

marcelm avatar marcelm commented on August 28, 2024 1

The warnings are gone... success!!! =) As always, thank you very much for fixing this so quickly!!

Perfect, thanks for testing!

Out of curiosity, do you know why the editable installation changed the version to the previous 0.10.1.dev instead of 1.0.0.dev for example??

Yes. There are two things that come together here: First, we use setuptools_scm to automatically set the version number based on the most recent Git tag. 0.10.1.dev30 means: The most recent Git tag v0.10.0 plus 30 commits. In case you are wondering: 0.10.0 gets bumped to 0.10.1 automatically because .dev releases are considered to be earlier than releases without .dev (according to PEP 440), so without the bump, the version number would sort incorrectly. Second, I made a mistake when tagging the 1.0.0 release: The v1.0.0 tag is on a branch, that is, it is not an ancestor commit of the current HEAD. Instead, setuptools_scm detects v0.10.0 to be the most recent tag.

This will be fixed when we release 1.0.1.

from dnaio.

marcelm avatar marcelm commented on August 28, 2024 1

Yes. There are two things that come together here

I see, thanks for the explanation! I will be waiting for 1.0.1.

It’s released now :-)

And that reminds me, I saw this on 1.0.0: Added id and comment properties to SequenceRecord.... Great addition, I have been using a split from "record.name" to get those fields until now, but with that I will be able to get them directly.

Yes, same here, it got a bit annoying!

Many years ago, I used BioPerl extensively, and they had 2 methods to access the same thing:

while (my $seq_obj = $fasta->next_seq) {
    my $display_id = $seq_obj->display_id;
    my $desc       = $seq_obj->desc;

Good thing that dnaio will also have them now. =)

I think I got the idea from Biopython, which may in turn have been inspired by BioPerl ...

from dnaio.

marcelm avatar marcelm commented on August 28, 2024

What are the typing problems in your own code when you try to use dnaio?

It is a good point that we should test the type annotations more. We currently only run mypy src/ in our own tests. Extending this to cover the tests/ directory makes some sense. However, note that mypy by default only checks functions that have some form of type annotation,so most test functions will be ignored (which is why you don’t get more errors).

That said, running mypy on the root of the repository as you did tests too much because it also includes the Sphinx configuration file doc/conf.py, for example, but that is not interesting.

Also, I may not necessarily want bytes_ascii_check to be type annotated because it is an implementation detail. Externally, that is when typechecking your own code that uses dnaio, you should not see this function appearing as a problem.

Regarding PEP 8, note that we currently use flake8 and have configured it to allow a line length of 99 characters, see tox.ini. This isn’t anything that should give you error messages within your own project.

from dnaio.

fjossandon avatar fjossandon commented on August 28, 2024

Hello again,

What are the typing problems in your own code when you try to use dnaio?

For example, this test code I just made:

from pathlib import Path

import dnaio


def process_fasta(folder: str, in_filename: str) -> None:
    input_file = Path(folder, in_filename)
    with dnaio.open(str(input_file)) as reader:
        for record in reader:
            print(record)


folder = "/home/fossandon/Documents/temp"
in_filename = "test.fa"
process_fasta(folder, in_filename)

It reads a small fasta and prints to the screen. This is what happens:

$ python3 test2.py
<SequenceRecord(name='sequence_id dummy', sequence='AAAAAAAAAAAAAAAAAAAAAAAA')>

$ pycodestyle test2.py

$ mypy test2.py
test2.py:8: error: Item "SingleEndReader" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__enter__"
test2.py:8: error: Item "PairedEndReader" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__enter__"
test2.py:8: error: Item "SingleEndWriter" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__enter__"
test2.py:8: error: Item "PairedEndWriter" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__enter__"
test2.py:8: error: Item "MultipleFileWriter" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__enter__"
test2.py:8: error: Item "SingleEndReader" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__exit__"
test2.py:8: error: Item "PairedEndReader" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__exit__"
test2.py:8: error: Item "SingleEndWriter" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__exit__"
test2.py:8: error: Item "PairedEndWriter" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__exit__"
test2.py:8: error: Item "MultipleFileWriter" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "__exit__"
Found 10 errors in 1 file (checked 1 source file)

So, although I'm only importing it, it looks like Mypy goes into the dnaio function and scans it too. =/
If I remove the typing of my file completely (def process_fasta(folder, in_filename):), the warnings go away:

$ mypy test2.py
Success: no issues found in 1 source file

I'm not sure why, could it be because I have an editable installation of dnaio v0.10??? I did it because I'm also using Python 3.6.9 in this environment, and dnaio is only available until 0.7.1 with the normal installation:

$ pip3 list | grep dnaio
dnaio                      0.10.1.dev2+geea6a5d.d20230322 /home/fossandon/Documents/Github_repos/dnaio/src

$ pip3 install dnaio==0.10.0
Defaulting to user installation because normal site-packages is not writeable
ERROR: Could not find a version that satisfies the requirement dnaio==0.10.0 (from versions: 0.1, 0.2, 0.3, 0.4, 0.4.1, 0.4.2, 0.4.3, 0.4.4, 0.5.0, 0.5.1, 0.5.2, 0.6.0rc1, 0.6.0, 0.7.0, 0.7.1)
ERROR: No matching distribution found for dnaio==0.10.0

##########

That said, running mypy on the root of the repository as you did tests too much because it also includes the Sphinx configuration file doc/conf.py, for example, but that is not interesting.

My engineering colleague sometimes doesn't want or can't type check a line, and he uses a "type ignore" comment, like this, in case you find it useful. =)

    def __copy(self, v: _T) -> _T:
        return v.copy() if hasattr(v, "copy") else v  # type: ignore

##########

Regarding PEP 8, note that we currently use flake8 and have configured it to allow a line length of 99 characters, see tox.ini. This isn’t anything that should give you error messages within your own project.

Ah, I see, you also ignore the E203 there. I hadn't looked at that file before.

from dnaio.

marcelm avatar marcelm commented on August 28, 2024
def process_fasta(folder: str, in_filename: str) -> None:
    input_file = Path(folder, in_filename)
    with dnaio.open(str(input_file)) as reader:

It’s unrelated to this issue, but note that dnaio.open() accepts Path objects directly, so you can write this as with dnaio.open(input_file) as reader:.

$ mypy test2.py
test2.py:8: error: Item "SingleEndReader" of "Union[SingleEndReader, PairedEndReader, SingleEndWriter, PairedEndWriter, MultipleFileReader, MultipleFileWriter]" has no attribute "enter"
[...]

Thanks, I fixed these today and opened PR #120.

So, although I'm only importing it, it looks like Mypy goes into the dnaio function and scans it too.

Yes, that’s how mypy works: It looks at the files that you import and uses the type annotations that exist in those files. And in the case of dnaio, the annotations were incomplete.

If I remove the typing of my file completely (def process_fasta(folder, in_filename):), the warnings go away:

That is also expected. By default, mypy only typechecks functions that have some type annotations in their signature. This allows for gradually adding type hints to a project: You start with all functions being unannotated and then add type hints function by function, and mypy only checks those functions that have already been worked on.

I did it because I'm also using Python 3.6.9 in this environment, and dnaio is only available until 0.7.1 with the normal installation:

Yes, we dropped Python 3.6 support after the release of dnaio 0.7.1. Both Python 3.6 and 3.7 are end-of-life and shouldn’t be used anymore because they don’t get security fixes. If your Python 3.6 comes from a distribution that still gets security fixes, then it’s the distribution’s responsibility, and you can continue using it, but then combining an old Python with the most recent dnaio version is up to you. Good that it seems to work in this case.

My engineering colleague sometimes doesn't want or can't type check a line, and he uses a "type ignore" comment, like this, in case you find it useful. =)

    def __copy(self, v: _T) -> _T:
        return v.copy() if hasattr(v, "copy") else v  # type: ignore

Yes, we’ve used a couple of these type ignore comments. My point was more that when you run mypy on your project, it parses the installed files only (those that pip install puts in site-packages/). The setup.py or the Sphinx configuration conf.py are not actually installed and will never cause errors for you (when you typecheck your project). And the easiest way to not get errors when we run mypy on dnaio itself is that we just don’t run mypy on them.

Could you possibly test #120 (branch typing) and see whether you still get any complaints from mypy when you run it on your project/script?

from dnaio.

fjossandon avatar fjossandon commented on August 28, 2024

It’s unrelated to this issue, but note that dnaio.open() accepts Path objects directly, so you can write this as with dnaio.open(input_file) as reader:.

Ah, excellent!

Yes, we dropped Python 3.6 support after the release of dnaio 0.7.1. Both Python 3.6 and 3.7 are end-of-life and shouldn’t be used anymore because they don’t get security fixes. If your Python 3.6 comes from a distribution that still gets security fixes, then it’s the distribution’s responsibility, and you can continue using it, but then combining an old Python with the most recent dnaio version is up to you. Good that it seems to work in this case.

Yes, it's an old machine I have, which I have not bothered to update to the latest Ubuntu, and I kind of hacked my way to still update dnaio. I do have a newer machine but I have not yet fully migrated my work to it (and it has a not-charging battery issue).

The setup.py or the Sphinx configuration conf.py are not actually installed and will never cause errors for you (when you typecheck your project). And the easiest way to not get errors when we run mypy on dnaio itself is that we just don’t run mypy on them.

I see what you mean.

Could you possibly test #120 (branch typing) and see whether you still get any complaints from mypy when you run it on your project/script?

For sure!
dnaio 1.0.0 output:

## dnaio 1.0.0
$ pip install dnaio==1.0.0
Requirement already satisfied: dnaio==1.0.0 in /home/francisco/pyenvs/s3utils/lib/python3.11/site-packages (1.0.0)
Requirement already satisfied: xopen>=1.4.0 in /home/francisco/pyenvs/s3utils/lib/python3.11/site-packages (from dnaio==1.0.0) (1.7.0)
Requirement already satisfied: isal>=1.0.0 in /home/francisco/pyenvs/s3utils/lib/python3.11/site-packages (from xopen>=1.4.0->dnaio==1.0.0) (1.3.0)

$ mypy test.py 
test.py:8: error: Item "SingleEndReader" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__enter__"  [union-attr]
test.py:8: error: Item "PairedEndReader" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__enter__"  [union-attr]
test.py:8: error: Item "SingleEndWriter" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__enter__"  [union-attr]
test.py:8: error: Item "PairedEndWriter" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__enter__"  [union-attr]
test.py:8: error: Item "MultipleFileWriter" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__enter__"  [union-attr]
test.py:8: error: Item "SingleEndReader" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__exit__"  [union-attr]
test.py:8: error: Item "PairedEndReader" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__exit__"  [union-attr]
test.py:8: error: Item "SingleEndWriter" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__exit__"  [union-attr]
test.py:8: error: Item "PairedEndWriter" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__exit__"  [union-attr]
test.py:8: error: Item "MultipleFileWriter" of "SingleEndReader | PairedEndReader | SingleEndWriter | PairedEndWriter | MultipleFileReader | MultipleFileWriter" has no attribute "__exit__"  [union-attr]
Found 10 errors in 1 file (checked 1 source file)

dnaio typing branch:

$ git checkout typing 
branch 'typing' set up to track 'origin/typing'.
Switched to a new branch 'typing'

$ pip install -e .
Obtaining file:///home/francisco/Documents/Github_repos/dnaio
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Installing backend dependencies ... done
  Preparing editable metadata (pyproject.toml) ... done
Requirement already satisfied: xopen>=1.4.0 in /home/francisco/pyenvs/s3utils/lib/python3.11/site-packages (from dnaio==0.10.1.dev30+g1c59d72) (1.7.0)
Requirement already satisfied: isal>=1.0.0 in /home/francisco/pyenvs/s3utils/lib/python3.11/site-packages (from xopen>=1.4.0->dnaio==0.10.1.dev30+g1c59d72) (1.3.0)
Building wheels for collected packages: dnaio
  Building editable for dnaio (pyproject.toml) ... done
  Created wheel for dnaio: filename=dnaio-0.10.1.dev30+g1c59d72-0.editable-cp311-cp311-linux_x86_64.whl size=3452 sha256=fc4b68f8865772537b6e1c47bebbe1bcea16a4b54dc3b2c55112cec978b176fb
  Stored in directory: /tmp/pip-ephem-wheel-cache-qo1hnat_/wheels/7a/70/44/37d219c06881e8e0c4e16157dd9801d6551b189d047d3b4b12
Successfully built dnaio
Installing collected packages: dnaio
  Attempting uninstall: dnaio
    Found existing installation: dnaio 1.0.0
    Uninstalling dnaio-1.0.0:
      Successfully uninstalled dnaio-1.0.0
Successfully installed dnaio-0.10.1.dev30+g1c59d72

$ mypy test.py 
Success: no issues found in 1 source file

The warnings are gone... success!!! =)
As always, thank you very much for fixing this so quickly!!

Out of curiosity, do you know why the editable installation changed the version to the previous 0.10.1.dev instead of 1.0.0.dev for example??

from dnaio.

fjossandon avatar fjossandon commented on August 28, 2024

Yes. There are two things that come together here

I see, thanks for the explanation! I will be waiting for 1.0.1.

And that reminds me, I saw this on 1.0.0: Added id and comment properties to SequenceRecord.... Great addition, I have been using a split from "record.name" to get those fields until now, but with that I will be able to get them directly.
Many years ago, I used BioPerl extensively, and they had 2 methods to access the same thing:

while (my $seq_obj = $fasta->next_seq) {
    my $display_id = $seq_obj->display_id;
    my $desc       = $seq_obj->desc;

Good thing that dnaio will also have them now. =)

from dnaio.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.