🚀 Feature: use fstring instead str.replace() ?

[dan-gut1]

🔖 Feature description

Why does the SDK uses str.replace() instead fstring?.

🎤 Pitch

In the past few days I'm working on implementing the front of appwrite in python for learning purposes and I've note that the 'python server sdk' mostly uses str.replace() for its calls. increasing processing time on each call of str.replace().

So it raise the question why these calls doesn't uses fstring?, it will greatly reduce processing time.

for example from Services.Databases:

 def update_document(self, database_id, collection_id, document_id, data = None, permissions = None):
        """Update Document"""

        
        path = '/databases/{databaseId}/collections/{collectionId}/documents/{documentId}'
        params = {}
        if database_id is None:
            raise AppwriteException('Missing required parameter: "database_id"')

        if collection_id is None:
            raise AppwriteException('Missing required parameter: "collection_id"')

        if document_id is None:
            raise AppwriteException('Missing required parameter: "document_id"')

        path = path.replace('{databaseId}', database_id)
        path = path.replace('{collectionId}', collection_id)
        path = path.replace('{documentId}', document_id)

        params['data'] = data
        params['permissions'] = permissions

        return self.client.call('patch', path, {
            'content-type': 'application/json',
        }, params)

Instead it is possible to use fstring, but need to note that there is lack of compliance with python under version 3.6.

 def update_document(self, database_id, collection_id, document_id, data = None, permissions = None):
        """Update Document"""

        
        params = {}
        if database_id is None:
            raise AppwriteException('Missing required parameter: "database_id"')

        if collection_id is None:
            raise AppwriteException('Missing required parameter: "collection_id"')

        if document_id is None:
            raise AppwriteException('Missing required parameter: "document_id"')
        
       # bare minimum cpu usage
        path = f'/databases/{databaseId}/collections/{collectionId}/documents/{documentId}'

        params['data'] = data
        params['permissions'] = permissions

        return self.client.call('patch', path, {
            'content-type': 'application/json',
        }, params)

👀 Have you spent some time to check if this issue has been raised before?

• [X] I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

• [X] I have read the Code of Conduct

[stnguyen90]

lack of compliance with python under version 3.6.

As you suggested, f strings are not as compatible so it might be better if we use str.format() instead of str.replace().

[dan-gut1]

So if so, I guess it well be better for write it with future f-string support as follow ?

'/databases/{databaseId}/collections/{collectionId}/documents/{documentId}'.format(databaseId=databaseId, collectionId=collectionId, documentId=documentId )

So in future time it will be easily change to native fstring?

As far as I saw fstring faster then string formating.

[GbotemiB]

Hi, I will like to work on this task.

[rahulsunil2]

I would like to work on this.

[stnguyen90]

@dan-gut1, let's go with str.format() approach. Would you like to work on this?

[dan-gut1]

Would you like to work on this?

I will be delighted, although there is enough work for all of us, if you would like to split the work among us no issue with that.

Can you please reference us to the, correct "HOW" to start contribution to code\python SDK repo?, and tests.

[stnguyen90]

@dan-gut1, best for only one person to work on this. Please refer to:

1. Template for service: https://github.com/appwrite/sdk-generator/blob/2637d47c7d1e5058a6f572078437f432bd082e71/templates/python/package/services/service.py.twig#L16
2. Instructions on how to contribute: https://github.com/appwrite/sdk-generator/blob/master/CONTRIBUTING.md
3. Script to generate SDKs: https://github.com/appwrite/sdk-generator/blob/master/example.php

Please reach out if you're stuck with anything.

[dan-gut1]

So in the middle way of making the changes, I was asked myself if these changes are worth doing, I made a little test, and its result not so good (at least in me opinion).

The test I made as follow with 3 variables.

1. Test the execution speed the current creation of the path using str.replace().
2. Test the execution speed the current creation of the path using str.format with variable assignment.
3. Test the execution speed the current creation of the path using str.format by variable indexing.
4. Test the execution speed the current creation of the path using fstring.

Here are brief result using timeit.timeit

replace path: 0.5664167 format with assignment: 0.9150742999999999
format with indexs: 0.5907963000000003
fstring path: 0.3794525000000002

you can find the test code here - https://pastebin.com/FtY1XXcW

So it raise a question.

Is the changing I'm working on worth doing?, although it make the path creation much more standard, I use format with assignment which is the worst time consuming. so I think that if we're not using fstring there's no point to continue with the modifications.

@stnguyen90 let me know what you thinking.

[stnguyen90]

@dan-gut1 heh...probably not since it's already working 😅

[ayaan-qadri]

Since f-string can be implement for version 3.6 or above instead we can use string concatenation as well, it support all python version and also more faster than all (still not faster than f-string)

Edit: Using ''.join we can achieve even more faster than concatenation (We can use it python version 1.6 or above)

replace path: 0.49085860000923276 format with assignment: 0.7151281999831554 format with index: 0.3822980000113603 fstring path: 0.15623890000279061 concatenation path: 0.24857970001176 join path: 0.19559379998827353

Here is the added string concatenation and join method performance check code in the dan-gut1's code: https://pastebin.com/XRPJpHRi