Make add_index public

[aMahanna]

This PR proposes to switch _add_index to add_index, and to deprecate the add_*_index methods in favour of generalizing to add_index (similar to how we have create_view, create_analayzer, etc.)

However, we need to decide if:

• The data parameter of add_index should expect camelCase or snake_case fields

The current add_*_index methods accept snake_case, convert to camelCase for API purposes, then output snake_case. We need to decide if we want to follow suit for add_index, or if we want to drop this whole snake_case to camelCase to snake_case conversion.

[aMahanna]

hey @apetenchea could I get your thoughts on this (PR description) ^

wondering what the best way to proceed is

[apetenchea]

@aMahanna

Since the opportunity is now there, let's drop the snake_case to camelCase conversion for the new add_index. This is not an easy decision, as it may seem inconvenient at first to some users. But, because this release changes the API anyways, let's take full advantage of it.

My rationale is, if we skip the conversion and just pass the arguments as they are, any additional index parameters that is going to be added in future ArangoDB releases will be supported automatically by our driver. Having that extra bit of validation is nice, but the HTTP API and the documentation are already so simple that I'm afraid the entire snake_case thing creates confusion sometimes. It's the pythonic way, I know, but our driver is really a convenient wrapper on top of the requests API.

In the end, our users want to use the database from Python. Let's also specify in the docstring of data that it's supposed to be used exactly as in the official ArangoDB HTTP docs (eg camelCase).

What do you think?

[aMahanna]

@apetenchea

I can get behind this change, but it raises the question of what should we do with formatter.py, as that is what is currently driving the snake_case to camelCase conversion logic.

The way I see it, we have two "snake_case vs camelCase" topics;

1. Should dict parameters of methods that interact with the ArangoDB API contain keys that are snake_case or camelCase? i.e this would apply to more than just the add_index method

2. Should non-dict parameters of methods that interact with the ArangoDB API be snake_case or camelCase? e.g requestTimeout vs request_timeout

I apologize for widening the scope, but it's a thought that just occurred to me now after reading your proposal

Happy to move this convo somewhere else if preferred

My takeaway is:

• If the answer is camelCase to both 1 and 2, then that would imply a much larger PR

• If the answer is camelCase for 1 but not 2, then another PR would also be required to address other dict-based parameters

• However if the answer is camelCase for just the add_index method, then we would be introducing some (albeit small) UX inconsistency in the driver

• The alternative would be to continue maintaining the snake_case to camelCase to snake_case logic

[apetenchea]

@aMahanna
You're right - this issue largely stems from the differing conventions between Python (predominantly snake_case) and JavaScript (camelCase), not to mention our primary use of camelCase in the ArangoDB C++ codebase. Given that the Python driver was initially a community-led project adhering to PEP8 guidelines, the prevalence of snake_case parameters among our Python users is understandable. Some IDEs would even emit a small warning when camelCase is used for this purpose.

For dictionary parameters, aligning with camelCase would ideally mirror the 1-1 mapping with our HTTP API, reducing confusion. However, the practicality of retroactively changing this across all methods poses a significant challenge, particularly in terms of user inconvenience and the potential for breaking backward compatibility. While achieving uniformity across all dict-based parameters is conceptually appealing, that might have implications for our users' existing codebases.

Regarding non-dict parameters, the impact of transitioning to camelCase would be even more profound, necessitating widespread changes on the user's end. A gradual approach, initially supporting both snake_case and camelCase conventions while providing ample warning, and then eventually switching off the legacy one, might mitigate backlash, but this is not without its own complexities.

I propose a cautious approach: eliminate snake_case conversion for the newly added add_index method while maintaining current conventions for legacy methods, using the formatter upon return, but just for the methods that were already there (eg add_hash_index) such that we keep the old stuff as is, while inviting everyone to adapt their code a little to the new method. The potential UX inconsistency (adopting camelCase for just the add_index method) makes me scratch my head too (see Database::create_view for example), but I think such choices were made under constraints like development time, which goes to show that indeed things are much simpler when sticking to the same convention as the database API (in our case, camel). This is chance to do it incrementally, without too many breaking changes in one release - let's see if the community reacts in some way.

[aMahanna]

@apetenchea

agreed! moving forward with this will give us a chance to get community feedback

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 12.50000% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 95.95%. Comparing base (a26e06a) to head (8c1ef8e).

Files	Patch %	Lines
arango/collection.py	12.50%	21 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #321      +/-   ##
==========================================
- Coverage   98.22%   95.95%   -2.28%     
==========================================
  Files          26       26              
  Lines        4283     4277       -6     
==========================================
- Hits         4207     4104     -103     
- Misses         76      173      +97     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[apetenchea]

The add_index takes a data parameter - the keys are pure ArangoDB REST API arguments, no snake_case conversion anymore. However, for the output, the legacy methods remain the same, while the new add_index method by default skips formatting.

I have added a generous note in the documentation explaining that. Hopefully, there should be no confusion among the users, as they switch to the add_index method.

[aMahanna]

LGTM! just two small doc fixes