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

Export property types #65

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Export property types #65

wants to merge 9 commits into from

Conversation

jlheflin
Copy link
Contributor

@jlheflin jlheflin commented Sep 6, 2024

I didn't know if the addition of property types needed to be added like they were in chemist, but I think those are for making modules.

Copy link
Contributor

@ryanmrichard ryanmrichard left a comment

Choose a reason for hiding this comment

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

Looks like it's close to being r2g.

#include <ghostfragment/ghostfragment.hpp>
#include <pluginplay/pluginplay.hpp>

namespace ghostfragment {
namespace ghostfragment::pt {
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes the namespace the ghostfragment python module is extracted into. We want it to be extracted into the ghostfragment namespace, not ghostfragment::pt.

Suggested change
namespace ghostfragment::pt {
namespace ghostfragment {

Comment on lines 33 to 50
EXPORT_PROPERTY_TYPE(CappedFragments, m);
// EXPORT_PROPERTY_TYPE(pt::CappedFragmentsTraits, m);
EXPORT_PROPERTY_TYPE(NuclearGraphToFragments, m);
// EXPORT_PROPERTY_TYPE(pt::NuclearGraphToFragmentsTraits, m);
EXPORT_PROPERTY_TYPE(NuclearGraph, m);
// EXPORT_PROPERTY_TYPE(pt::NuclearGraphTraits, m);
EXPORT_PROPERTY_TYPE(FragmentedNuclei, m);
// EXPORT_PROPERTY_TYPE(pt::FragmentedNucleiTraits, m);
EXPORT_PROPERTY_TYPE(FragmentedChemicalSystem, m);
// EXPORT_PROPERTY_TYPE(pt::FragmentedChemicalSystemTraits, m);
EXPORT_PROPERTY_TYPE(FragmentWeights, m);
// EXPORT_PROPERTY_TYPE(pt::FragmentWeightsTraits, m);
EXPORT_PROPERTY_TYPE(ConnectivityTable, m);
// EXPORT_PROPERTY_TYPE(pt::ConnectivityTableTraits, m);
EXPORT_PROPERTY_TYPE(BrokenBonds, m);
// EXPORT_PROPERTY_TYPE(pt::BrokenBondsTraits, m);
EXPORT_PROPERTY_TYPE(Intersections, m);
// EXPORT_PROPERTY_TYPE(pt::IntersectionTraits, m);
Copy link
Contributor

@ryanmrichard ryanmrichard Sep 6, 2024

Choose a reason for hiding this comment

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

Remove the comments and add back the pt namespace.

Suggested change
EXPORT_PROPERTY_TYPE(CappedFragments, m);
// EXPORT_PROPERTY_TYPE(pt::CappedFragmentsTraits, m);
EXPORT_PROPERTY_TYPE(NuclearGraphToFragments, m);
// EXPORT_PROPERTY_TYPE(pt::NuclearGraphToFragmentsTraits, m);
EXPORT_PROPERTY_TYPE(NuclearGraph, m);
// EXPORT_PROPERTY_TYPE(pt::NuclearGraphTraits, m);
EXPORT_PROPERTY_TYPE(FragmentedNuclei, m);
// EXPORT_PROPERTY_TYPE(pt::FragmentedNucleiTraits, m);
EXPORT_PROPERTY_TYPE(FragmentedChemicalSystem, m);
// EXPORT_PROPERTY_TYPE(pt::FragmentedChemicalSystemTraits, m);
EXPORT_PROPERTY_TYPE(FragmentWeights, m);
// EXPORT_PROPERTY_TYPE(pt::FragmentWeightsTraits, m);
EXPORT_PROPERTY_TYPE(ConnectivityTable, m);
// EXPORT_PROPERTY_TYPE(pt::ConnectivityTableTraits, m);
EXPORT_PROPERTY_TYPE(BrokenBonds, m);
// EXPORT_PROPERTY_TYPE(pt::BrokenBondsTraits, m);
EXPORT_PROPERTY_TYPE(Intersections, m);
// EXPORT_PROPERTY_TYPE(pt::IntersectionTraits, m);
using namespace pt;
EXPORT_PROPERTY_TYPE(CappedFragments, m);
EXPORT_PROPERTY_TYPE(NuclearGraphToFragments, m);
EXPORT_PROPERTY_TYPE(NuclearGraph, m);
EXPORT_PROPERTY_TYPE(FragmentedNuclei, m);
EXPORT_PROPERTY_TYPE(FragmentedChemicalSystem, m);
EXPORT_PROPERTY_TYPE(FragmentWeights, m);
EXPORT_PROPERTY_TYPE(ConnectivityTable, m);
EXPORT_PROPERTY_TYPE(BrokenBonds, m);
EXPORT_PROPERTY_TYPE(Intersections, m);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this previously and ended up with a syntax error within python due to the double colon.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. That's my bad. Try my edit.

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