Skip to content

Consistent constants#419

Draft
Joseph-Edwards wants to merge 3 commits intolibsemigroups:mainfrom
Joseph-Edwards:consistent-constants
Draft

Consistent constants#419
Joseph-Edwards wants to merge 3 commits intolibsemigroups:mainfrom
Joseph-Edwards:consistent-constants

Conversation

@Joseph-Edwards
Copy link
Copy Markdown
Collaborator

This PR includes some minor quality-of-life improvements such as removing redundant aliases, changing parameters to const-references, and using the from_int function more.

Comment thread src/transf.cpp
Comment on lines 169 to 171
[](auto val) -> int_or_unsigned_constant<Scalar> {
if (val != UNDEFINED) {
return {val};
}
return {UNDEFINED};
return from_int(val);
});
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's possible to remove this lambda all together, with something like

static_cast<int_or_constant<Scalar> (*)(Scalar)>(from_int)

but this felt a bit unwieldy to me, so I opted to keep the lambda.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And you can't just replace the lambda with from_int ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately not; transform is unable to infer the type of from_int. I think this is because from_int is an overloaded function template. Something like this seems to work:

auto r = rx::iterator_range(self.begin(), self.end())
         | rx::transform<int_or_constant<Scalar> (*)(Scalar)>(
             &from_int);

Should we go with that instead?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nah, the lambda is better. Sorry for the noise.

@Joseph-Edwards Joseph-Edwards marked this pull request as draft April 23, 2026 16:49
@Joseph-Edwards
Copy link
Copy Markdown
Collaborator Author

This is a draft until I can get the tests passing

@james-d-mitchell
Copy link
Copy Markdown
Member

This is a draft until I can get the tests passing

I don't see anything obviously wrong with the changes in this pr, I wonder what's going on with the ci

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