Skip to content

[ENH] Remove Structure support for packaged pdbs#209

Open
satvshr wants to merge 18 commits intomainfrom
issue208
Open

[ENH] Remove Structure support for packaged pdbs#209
satvshr wants to merge 18 commits intomainfrom
issue208

Conversation

@satvshr
Copy link
Collaborator

@satvshr satvshr commented Dec 1, 2025

stacks on #198
fixes #208
Now that we are using MoleculeLoader as our default in-memory format, we can remove the Structure loader for all packaged pdbs. We are not completely removing Structure support, though, as we still have functions in utils to convert from pdb to struct and struct to aaseq.

@satvshr satvshr requested a review from fkiraly December 1, 2025 18:31

def load_1gnh_structure(pdb_path=None):
# This function is provided only to test struct_to_aaseq.
def _load_1gnh_structure(pdb_path=None):
Copy link
Collaborator Author

@satvshr satvshr Dec 23, 2025

Choose a reason for hiding this comment

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

Function kept as it is used for testing struct_to_aaseq

Copy link
Collaborator

Choose a reason for hiding this comment

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

I might not be in loop with this one, but Could you confirm we are removing this becuase it's no longer needed because of Moleculeloader but the loader currently doesn't have the struct_to_aaseq function? So we need this to test it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems a bit counterintutive, I might not be part of the design discussion or might have simply forgot, but can we use moleculeloader to simply do that and either get rid of this structure way entirely, or keep the structure way, as is , for all the molecules if it is needed?

@satvshr satvshr requested a review from siddharth7113 January 8, 2026 13:43
@siddharth7113
Copy link
Collaborator

Just to clarify we are also adding a new pdb 1BRQ in this PR, right? Is this also needed from ecoSPECS teams?

@satvshr
Copy link
Collaborator Author

satvshr commented Jan 15, 2026

Just to clarify we are also adding a new pdb 1BRQ in this PR, right?

It stacks on the 1BRQ PR (as mentioned in the PR description)

Is this also needed from ecoSPECS teams?

No, from the PR description: "Now that we are using MoleculeLoader as our default in-memory format, we can remove the Structure loader for all packaged pdbs." It is just clean up nothing else.

@siddharth7113
Copy link
Collaborator

siddharth7113 commented Jan 15, 2026

Just to clarify we are also adding a new pdb 1BRQ in this PR, right?

It stacks on the 1BRQ PR (as mentioned in the PR description)

Is this also needed from ecoSPECS teams?

No, from the PR description: "Now that we are using MoleculeLoader as our default in-memory format, we can remove the Structure loader for all packaged pdbs."

My bad, I read the wrong name and assume it was other pdb, I just reviewed #198

@satvshr
Copy link
Collaborator Author

satvshr commented Feb 13, 2026

@siddharth7113 do you remember what we concluded from that discussion?

@siddharth7113
Copy link
Collaborator

Yeah, there was some confusion regarding retaining some function, but it was ultimately being used in a test , everything else looks good to me

@siddharth7113
Copy link
Collaborator

Also this stack #198 which @fkiraly needs to approve, then we can merge this one,

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.

[ENH] Remove Structure support for packaged pdbs

2 participants