Skip to content

Support native types in macros#1511

Merged
davidism merged 1 commit intopallets:mainfrom
mkrizek:issue-1510
Nov 10, 2021
Merged

Support native types in macros#1511
davidism merged 1 commit intopallets:mainfrom
mkrizek:issue-1510

Conversation

@mkrizek
Copy link
Copy Markdown
Contributor

@mkrizek mkrizek commented Oct 13, 2021

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@mkrizek mkrizek mentioned this pull request Oct 13, 2021
@davidism
Copy link
Copy Markdown
Member

I like this second approach of defining concat on Environment. Maybe it would be better to do concat = str.join or concat = native_concat instead of adding an extra method call.

@mkrizek
Copy link
Copy Markdown
Contributor Author

mkrizek commented Oct 18, 2021

I like this second approach of defining concat on Environment. Maybe it would be better to do concat = str.join or concat = native_concat instead of adding an extra method call.

Yeah, that is what I had initially but got TypeError: native_concat() takes 1 positional argument but 2 were given when used that way. Looking into it a bit more it appears that doing concat = partial(native_concat) works though, I have not looked into why yet.

Still playing with this, thanks for the feedback!

@davidism
Copy link
Copy Markdown
Member

It's a problem with Python functions defined in C vs Python. str.join is a C function, but since native_concat is Python it requires concat = staticmethod(native_concat). It's a bit annoying, but I don't expect this to be a public API (beyond "assign this to make a custom rendering system").

@mkrizek mkrizek marked this pull request as ready for review October 20, 2021 11:49
@davidism davidism added this to the 3.1.0 milestone Nov 9, 2021
@davidism davidism merged commit 7d72eb7 into pallets:main Nov 10, 2021
@mkrizek mkrizek deleted the issue-1510 branch November 11, 2021 15:44
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native types for macros

2 participants