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

Chain.jl #28

Open
wants to merge 44 commits into
base: main
Choose a base branch
from
Open

Chain.jl #28

wants to merge 44 commits into from

Conversation

ana-bblima
Copy link

Add the capacity of iterating over chains in a pdb file with PDBTools.

src/Chain.jl Outdated Show resolved Hide resolved
src/Chain.jl Show resolved Hide resolved
src/Chain.jl Show resolved Hide resolved
src/Chain.jl Show resolved Hide resolved
test/protein_test.pdb Outdated Show resolved Hide resolved
src/PDBTools.jl Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 22, 2024

Codecov Report

Attention: Patch coverage is 96.82540% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.72%. Comparing base (b1511b7) to head (0032df5).

Files with missing lines Patch % Lines
src/Chain.jl 96.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #28      +/-   ##
==========================================
+ Coverage   86.09%   86.72%   +0.63%     
==========================================
  Files          23       24       +1     
  Lines         985     1047      +62     
==========================================
+ Hits          848      908      +60     
- Misses        137      139       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ana-bblima ana-bblima requested a review from lmiq December 7, 2024 19:41
Project.toml Outdated Show resolved Hide resolved
@@ -1,5 +1,5 @@
"""
Residue(atoms::AbstractVector{<:Atom}, range::UnitRange{Int})
Residue(atoms::AbstractVector{<:Atom}, range::UnitRange{Int})
Copy link
Member

Choose a reason for hiding this comment

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

Aqui por algum motivo você tirou um espaço e a identação está errada.

Copy link
Member

Choose a reason for hiding this comment

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

rever

@@ -31,6 +31,9 @@ julia> residues[5].chain
julia> residues[8].range
52:58

julia> mass(residues[1])
Copy link
Member

Choose a reason for hiding this comment

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

Não sei se aqui vai dar erro, mas testes com números de ponto flutuante (floats) em jldoctest são dor de cabeça, porque se qualquer algarismo variar o teste falha. Os jldoctests comparam exatamente o output gerado com o output esperado, e comparações de igualdade de floats nunca funcionam bem (teria que usar uma comparação aproximada).

Copy link
Member

Choose a reason for hiding this comment

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

rever

@@ -0,0 +1,146 @@
HEADER PDBTools.jl - 144 atoms 22-Nov-24
Copy link
Member

Choose a reason for hiding this comment

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

Este PDB ainda tem 1 modelo só e um segmento só. Arrisco dizer que podemos ter problemas nos casos em que isso não é assim.

Copy link
Member

Choose a reason for hiding this comment

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

não reparei se isto mudou no arquivo

@test name(chains[3]) == "C"
@test index.(filter(at -> resname(at) == "ASP" && name(at) == "CA", chains[1])) == [2]
@test length(findall(at -> resname(at) == "GLN", chains[1])) == 17
@test mass(chains[1]) == 353.37881000000016
Copy link
Member

Choose a reason for hiding this comment

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

Comparações de floats tem que sempre ser feitas com aproximadamente (\approx + TAB gera o símbolo de aproximadamente - ou isapprox(x,y).

Copy link
Member

Choose a reason for hiding this comment

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

corrigir este teste da massa para usar approx

julia> ats = read_pdb(PDBTools.CHAINSPDB);

julia> chains = collect(eachchain(ats))
Array{Chain,1} with 3 chains.
Copy link
Member

Choose a reason for hiding this comment

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

Isto não tínhamos mudado para ˋtypeof(chain)ˋ na função, de forma que aparece ˋVector{Chain}ˋ no ˋshowˋ?

"""
Chain(atoms::AbstractVector{<:Atom}, range::UnitRange{Int})

Creates a Chain data structure. It contains two fields: `atoms` which is a vector of
Copy link
Member

Choose a reason for hiding this comment

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

não acho que faça sentido descrever o conteúdo dos campos da estrutura (sâo internos).

Base.length(chains::EachChain) = sum(1 for chain in chains)
Base.firstindex(chains::EachChain) = 1
Base.lastindex(chains::EachChain) = length(chains)
# Base.reverse(chains::EachChain) = reverse(chains)
Copy link
Member

Choose a reason for hiding this comment

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

remover estes metodos comentados

Copy link
Member

Choose a reason for hiding this comment

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

tem um arquivo "chains.pdb" e outro "protein_test.pdb". Não sei qual dos dois você quer manter, mas em todo caso chame ele de "chains.pdb", para deixar claro que é o que é usado nos testes das chains.

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.

2 participants