Skip to content

Conversation

@B1ueber2y
Copy link
Contributor

This feature was added in #3465 but never implemented.

Comment on lines +219 to +220
std::string_view (*to_string_fn)(EnumT),
EnumT (*from_string_fn)(std::string_view),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not std::function<EnumT(std::string_view)>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In enum_utils.h we have overloaded the EnumTypeToString function with both int and EnumType, and because of this using std::function always throws.

To make it work we need to do static_cast<std::string_view(*)(FeatureExtractorType)>(FeatureExtractorTypeToString) at each call, which is very inconvenient.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should not overload but use a different name? E.g. FeatureExtractorTypeToString and FeatureExtractorIntTypeToString ?

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 think the overloading is quite conveient in practice. Lets keep it as it is now?

};

// Register as a string option pointing to our storage
AddDefaultOption(name, &info->value, help_text);
Copy link
Contributor

@ahojnnes ahojnnes Dec 29, 2025

Choose a reason for hiding this comment

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

Wondering whether we can use the existing enum macros to automatically compile a help text with all the available enum values as "{VALUE_A, VALUE_B, ...}"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is possible by adding another macro (e.g., ``xxxTypeValuesString) in enum_utils. However, it s a bit tricky what we should do for UNDEFINED and INVALID. It s unclear to me whether just filtering them is the right approach because they may be useful.

@B1ueber2y B1ueber2y merged commit 7eddd91 into colmap:main Dec 30, 2025
14 checks passed
@B1ueber2y B1ueber2y deleted the features/enum_option_manager branch December 30, 2025 16:43
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