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

Improve libcpp.unordered_map API #2168

Merged
merged 1 commit into from
Apr 12, 2018
Merged

Improve libcpp.unordered_map API #2168

merged 1 commit into from
Apr 12, 2018

Conversation

vallsv
Copy link
Contributor

@vallsv vallsv commented Mar 21, 2018

Hi.

This PR improve the API provided by libcpp unordered_map.

I only add reserve and bucket_count to the API cause it was needed on my side. But there is other missing functions. Let me know if you prefer a patch with the exhaustive API.

- Add `reserve` and `bucket_count` to the libcpp unordered_map API
@scoder
Copy link
Contributor

scoder commented Mar 21, 2018

Well, adding more than you strictly need at a given time would be nice towards a) the Cython project and b) others who want to use the features in the future.

@scoder
Copy link
Contributor

scoder commented Mar 21, 2018

Also note that tests would help to assure that this does not get broken by future changes. See the existing test files in tests/run/ and start the test runner with runtests.py cpp_stl.

@scoder
Copy link
Contributor

scoder commented Apr 8, 2018

Any update? Would be nice to get this in.

@vallsv
Copy link
Contributor Author

vallsv commented Apr 11, 2018

I have to work on it. This week or the next one.
But i will not have time to add unittests.

@scoder
Copy link
Contributor

scoder commented Apr 12, 2018

But i will not have time to add unittests.

Interesting attitude. You expect us to spend our time on maintaining the entire non-trivial tool that you use, but the maximum time that you are willing to spend on adding the features that you need is the time to drop them on us in the least maintainable way.

@scoder scoder merged commit f1c21d8 into cython:master Apr 12, 2018
@scoder scoder added this to the 0.28.2 milestone Apr 12, 2018
@vallsv
Copy link
Contributor Author

vallsv commented Apr 13, 2018

You kiding me? I expect nothing, I only propose a patch cause i think it could be useful for the community.
I anyway can't use the Cython master cause i have to use the version 0.21.1, then i have no interest in this merge. I inserted custom definitions on my own source files.

I don't have the infrastructure to test all the combination of compilators with C++98, C++11, C++16... plus my compilator which is supposed to support C++11, do not support all the signatures. Then i know it is a difficult task and i know i don't have the skill and the knowledge to do it. I am sorry, as most of people, I don't have an infinite amount of time, i just explain to you what i can do and when.

Thanks anyway for the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants