Selecting atoms with OBAtom indices given

Hi,

To select atoms when the OBAtom indices are given now use the code:

vector m_ligand;
QList<Primitive*> selectedAtoms;
for (j = m_ligand.begin(); j != m_ligand.end(); ++j)
selectedAtoms.append(static_cast<Atom*>(molecule->GetAtom(*j)));

widget->clearSelected();
widget->setSelected(selectedAtoms, true);
widget->update();

And to get the indices from a selection I use:

selectedAtoms = m_widget->selectedPrimitives();
for (int i = 0; i < selectedAtoms.size(); ++i) {
if (selectedAtoms[i]->type() == Primitive::AtomType) {
Atom *atom = static_cast<Atom *>(selectedAtoms[i]);
m_ligand.push_back(atom->index());
}
}

For this I also added “int index() { return GetIdx(); }” in the Atom
class (libavogadro/src/primitive.h)

Shall I move the first two to the GLWidget selection methods?
GLWidget::setSelected(QList selection)
QList GLWidget::selected();

I just want to make sure I place them in the right class, we have to
try and keep the code clean :slight_smile:

Tim

On Dec 8, 2007, at 3:44 PM, Tim Vandermeersch wrote:

Shall I move the first two to the GLWidget selection methods?
GLWidget::setSelected(QList selection)
QList GLWidget::selected();

But now you’re making atoms into a special bit here. If I pass an
index, what if it’s an index for a bond, or a residue, or …

Do you really think there’s a huge savings of code by the two methods
you’ve proposed? If so, you certainly want to make sure Atom is part
of the name, so it’s clear that these are methods for atom indexes,
and not something else.

Cheers,
-Geoff

On Dec 8, 2007 9:54 PM, Geoffrey Hutchison geoff.hutchison@gmail.com wrote:

On Dec 8, 2007, at 3:44 PM, Tim Vandermeersch wrote:

Shall I move the first two to the GLWidget selection methods?
GLWidget::setSelected(QList selection)
QList GLWidget::selected();

But now you’re making atoms into a special bit here. If I pass an
index, what if it’s an index for a bond, or a residue, or …

Do you really think there’s a huge savings of code by the two methods
you’ve proposed? If so, you certainly want to make sure Atom is part
of the name, so it’s clear that these are methods for atom indexes,
and not something else.

It would make my life easier to have them :-), but do you prefer to
keep the classes very basic and implement everything in the
extention/tool itself (or trough OB)?

And you areright about the name, if we add it, we should call
setSelectesAtoms(QList selection);

Tim

::selected seems pointless to me as there is really no benefit of
getting a list of integers over a list of Primitives. On top of that
you’re doing double work because to get the list of selected indices
you’ll have to traverse the list of selected primitives.

I also double Geoff’s concerns. I’m definitely happy to add functions
when needed but at this point we want to avoid cluttering the API.


Donald

(Sat, Dec 08, 2007 at 10:01:04PM +0100) Tim Vandermeersch tim.vandermeersch@gmail.com:

On Dec 8, 2007 9:54 PM, Geoffrey Hutchison geoff.hutchison@gmail.com wrote:

On Dec 8, 2007, at 3:44 PM, Tim Vandermeersch wrote:

Shall I move the first two to the GLWidget selection methods?
GLWidget::setSelected(QList selection)
QList GLWidget::selected();

But now you’re making atoms into a special bit here. If I pass an
index, what if it’s an index for a bond, or a residue, or …

Do you really think there’s a huge savings of code by the two methods
you’ve proposed? If so, you certainly want to make sure Atom is part
of the name, so it’s clear that these are methods for atom indexes,
and not something else.

It would make my life easier to have them :-), but do you prefer to
keep the classes very basic and implement everything in the
extention/tool itself (or trough OB)?

And you areright about the name, if we add it, we should call
setSelectesAtoms(QList selection);

Tim


SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It’s the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php


Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/avogadro-devel

On Dec 9, 2007 7:28 PM, Donald Ephraim Curtis d@milkbox.net wrote:

::selected seems pointless to me as there is really no benefit of
getting a list of integers over a list of Primitives. On top of that
you’re doing double work because to get the list of selected indices
you’ll have to traverse the list of selected primitives.

Yes, it’s true that it would be double work. But the benefit I had in
mind was more towards new developers, but you can always copy paste it
and adjust to your needs…

I also double Geoff’s concerns. I’m definitely happy to add functions
when needed but at this point we want to avoid cluttering the API.

I agree. But the added index() to Atom in primitive.h is needed right?
Since this wasn’t added either, I just tought you didn’t have time yet
to create these functions.


Donald

(Sat, Dec 08, 2007 at 10:01:04PM +0100) Tim Vandermeersch tim.vandermeersch@gmail.com:

On Dec 8, 2007 9:54 PM, Geoffrey Hutchison geoff.hutchison@gmail.com wrote:

On Dec 8, 2007, at 3:44 PM, Tim Vandermeersch wrote:

Shall I move the first two to the GLWidget selection methods?
GLWidget::setSelected(QList selection)
QList GLWidget::selected();

But now you’re making atoms into a special bit here. If I pass an
index, what if it’s an index for a bond, or a residue, or …

Do you really think there’s a huge savings of code by the two methods
you’ve proposed? If so, you certainly want to make sure Atom is part
of the name, so it’s clear that these are methods for atom indexes,
and not something else.

It would make my life easier to have them :-), but do you prefer to
keep the classes very basic and implement everything in the
extention/tool itself (or trough OB)?

And you areright about the name, if we add it, we should call
setSelectesAtoms(QList selection);

Tim


SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It’s the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php


Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/avogadro-devel

There is no need to currently wrap every function that OpenBabel
provides… For example. You can already do Atom::GetIdx() which is an
OBAtom function. So ::index isn’t needed.


Donald

(Sun, Dec 09, 2007 at 10:26:01PM +0100) Tim Vandermeersch tim.vandermeersch@gmail.com:

On Dec 9, 2007 7:28 PM, Donald Ephraim Curtis d@milkbox.net wrote:

::selected seems pointless to me as there is really no benefit of
getting a list of integers over a list of Primitives. On top of that
you’re doing double work because to get the list of selected indices
you’ll have to traverse the list of selected primitives.

Yes, it’s true that it would be double work. But the benefit I had in
mind was more towards new developers, but you can always copy paste it
and adjust to your needs…

I also double Geoff’s concerns. I’m definitely happy to add functions
when needed but at this point we want to avoid cluttering the API.

I agree. But the added index() to Atom in primitive.h is needed right?
Since this wasn’t added either, I just tought you didn’t have time yet
to create these functions.


Donald

(Sat, Dec 08, 2007 at 10:01:04PM +0100) Tim Vandermeersch tim.vandermeersch@gmail.com:

On Dec 8, 2007 9:54 PM, Geoffrey Hutchison geoff.hutchison@gmail.com wrote:

On Dec 8, 2007, at 3:44 PM, Tim Vandermeersch wrote:

Shall I move the first two to the GLWidget selection methods?
GLWidget::setSelected(QList selection)
QList GLWidget::selected();

But now you’re making atoms into a special bit here. If I pass an
index, what if it’s an index for a bond, or a residue, or …

Do you really think there’s a huge savings of code by the two methods
you’ve proposed? If so, you certainly want to make sure Atom is part
of the name, so it’s clear that these are methods for atom indexes,
and not something else.

It would make my life easier to have them :-), but do you prefer to
keep the classes very basic and implement everything in the
extention/tool itself (or trough OB)?

And you areright about the name, if we add it, we should call
setSelectesAtoms(QList selection);

Tim


SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It’s the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php


Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/avogadro-devel

Hmm, I don’t know what I was doing the other day, but you are totally
right. replacing the index() with GetIdx() gives the same result.

Tim

On Dec 9, 2007 11:09 PM, Donald Ephraim Curtis d@milkbox.net wrote:

There is no need to currently wrap every function that OpenBabel
provides… For example. You can already do Atom::GetIdx() which is an
OBAtom function. So ::index isn’t needed.


Donald

(Sun, Dec 09, 2007 at 10:26:01PM +0100) Tim Vandermeersch tim.vandermeersch@gmail.com:

On Dec 9, 2007 7:28 PM, Donald Ephraim Curtis d@milkbox.net wrote:

::selected seems pointless to me as there is really no benefit of
getting a list of integers over a list of Primitives. On top of that
you’re doing double work because to get the list of selected indices
you’ll have to traverse the list of selected primitives.

Yes, it’s true that it would be double work. But the benefit I had in
mind was more towards new developers, but you can always copy paste it
and adjust to your needs…

I also double Geoff’s concerns. I’m definitely happy to add functions
when needed but at this point we want to avoid cluttering the API.

I agree. But the added index() to Atom in primitive.h is needed right?
Since this wasn’t added either, I just tought you didn’t have time yet
to create these functions.


Donald

(Sat, Dec 08, 2007 at 10:01:04PM +0100) Tim Vandermeersch tim.vandermeersch@gmail.com:

On Dec 8, 2007 9:54 PM, Geoffrey Hutchison geoff.hutchison@gmail.com wrote:

On Dec 8, 2007, at 3:44 PM, Tim Vandermeersch wrote:

Shall I move the first two to the GLWidget selection methods?
GLWidget::setSelected(QList selection)
QList GLWidget::selected();

But now you’re making atoms into a special bit here. If I pass an
index, what if it’s an index for a bond, or a residue, or …

Do you really think there’s a huge savings of code by the two methods
you’ve proposed? If so, you certainly want to make sure Atom is part
of the name, so it’s clear that these are methods for atom indexes,
and not something else.

It would make my life easier to have them :-), but do you prefer to
keep the classes very basic and implement everything in the
extention/tool itself (or trough OB)?

And you areright about the name, if we add it, we should call
setSelectesAtoms(QList selection);

Tim


SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It’s the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php


Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/avogadro-devel