Skip to content

ignore unused parameter 'buf' warning #283

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

Conversation

peter-urban
Copy link
Contributor

@peter-urban peter-urban commented Sep 20, 2022

I included <xtensor-python/xtensor_type_caster_base.hpp> into my code to let pybind11 automatically convert xtensor xarrays to numpy arrays.

However I get two "-wunused-parameter warnings" which seem to be straightforward to fix :-)

@peter-urban
Copy link
Contributor Author

Not sure, but the appveyor fail seems to be unrelated to my pull request?

@tdegeus
Copy link
Member

tdegeus commented Mar 2, 2023

Thanks! You could also just remove the name buf from the argument. Is there any reason to keep it?

@peter-urban
Copy link
Contributor Author

peter-urban commented Mar 8, 2023

I don't understand the code well enough to propose such a change.

To me this looks like a dummy function (just just returns true) to implement a sort of interface (but I don't know which one).
Removing the variable could thus break this interface.

That said: If you have a better understanding of the code-base and you say that we can remove the argument, it is of cause not problem for me to adapt the PR :-)

Copy link
Member

@tdegeus tdegeus left a comment

Choose a reason for hiding this comment

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

I meant this

@peter-urban peter-urban requested a review from tdegeus March 10, 2023 08:48
@tdegeus tdegeus merged commit 7cb644a into xtensor-stack:master Mar 10, 2023
@tdegeus
Copy link
Member

tdegeus commented Mar 10, 2023

Tanks!

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