-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adapt Mia to populse_db V3 (#17) #18
base: master
Are you sure you want to change the base?
Conversation
- populse_mia.info
- populse_mia/__init__.py
- populse_mia/__main__.py - pre-commit run --all-files
- populse_mia/main.py
@sapetnioc , could you please review the populse_mia.data_manager.database_mia module and share your thoughts, especially regarding execution speed? Are there any suggestions to improve the performance of certain methods within this module? I'd really appreciate your feedback on this module, ASAP, because depending on what you think, I may need to change the current modifications for the rest of populse_mia! |
I see one point that could be improved but I do not really know the impact
on performance. It depends on the structure of the code that uses
DatabaseMia. Each method of DatabaseMia opens and closes the database
connection using "with". If you know that you are in a portion of code
where you will chain many small calls to DatabaseMia, it could be
interesting to be able to open the database once for all these calls and
close it at the end. The DatabaseMia API does not allow this. I recommend
to split it in three classes: DatabaseMia that can only be used to create
sessions using "with" keyword, a DatabaseMiaSchema that represents a
session with methods to modify schema and a DatabaseMiaData that represents
a session with methods to read/write data. Usage could be:
db = DatabaseMia(...)
with db.schema() as schema:
schema.add_collection(...)
schema.add_field(...)
with db.data(write=True) as data:
data.get_value(...)
data.set_value(...)
data.update_field_attributes(...) # field attributes are stored in data
After writing this, I see that add_collection needs both a shema and a data
session. Therefore my proposed optimization would require a change in
populse_db API to be able to get a data session from a schema session :
db = Storage(...)
with db.schema() as schema:
with schema.data():
...
Another option could be to keep schema related methods in DatabaseMia
methods without optimization (I suppose they require less optimization than
data methods).
…On Fri, Feb 7, 2025 at 12:22 PM Eric Condamine ***@***.***> wrote:
@sapetnioc <https://github.com/sapetnioc> , could you please review the
populse_mia.data_manager.database_mia
<https://github.com/populse/populse_mia/blob/75d9c45c0bcc1bc78d34fe12f19775dc636c7525/populse_mia/data_manager/database_mia.py>
module and share your thoughts, especially regarding execution speed? Are
there any suggestions to improve the performance of certain methods within
this module?
I'd really appreciate your feedback on this module, ASAP, because
depending on what you think, I may need to change the current modifications
for the rest of populse_mia!
—
Reply to this email directly, view it on GitHub
<#18 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXLUTSWD26VJEKGFN2M4PT2OSJEPAVCNFSM6AAAAABWMHUICCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNBSGYZTCMJZGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I modified populse_db to make it possible to get a data context from a schema context (I used session instead of context in my previous message but the right name for object used in "with" statement is context). The optimization I propose is not a big change (except that all code using database has to be modified to add "with") for one who already knows how to deal with contexts. I may help if you think it is worth to to this optimization. |
- populse_mia/utils/__init__.py - populse_mia/utils/utils.py: in progress
Thank you for your reply @sapetnioc . |
I understand the overall picture, but as the devil is in the detail, can we talk about this optimisation at the next meeting on Thursday? |
- populse_mia/utils/utils.py
No problem to talk about this. To try to make things clear, I implemented the API changes I have in mind in branch I also added a |
Thank you very much @sapetnioc. I've currently only skimmed the change in the |
- populse_mia/software_properties.py: in progress
- populse_mia/software_properties.py
- cleaning the database_mia module
- populse_mia/data_manager/project.py: in progress
- populse_mia/data_manager/project.py: in progress
- populse_mia/data_manager/project.py
- populse_mia/data_manager/filter.py
- populse_mia/data_manager/project_properties.py
- populse_mia/data_manager/data_loader.py: in progress
- populse_mia/data_manager/data_loader.py
- populse_mia/user_interface/pipeline_manager/type_editors.py
- populse_mia/user_interface/pipeline_manager/process_mia.py: in progress
- populse_mia/user_interface/pipeline_manager/process_mia.py: in progress
- populse_mia/user_interface/pipeline_manager/process_mia.py: in progress
- populse_mia/user_interface/pipeline_manager/process_mia.py:
- populse_mia/user_interface/pipeline_manager/node_controller.py
- populse_mia/user_interface/pipeline_manager/process_library.py: in progress
- populse_mia/user_interface/pipeline_manager/process_library.py: in progress
- populse_mia/user_interface/pipeline_manager/process_library.py: in progress
- populse_mia/user_interface/pipeline_manager/process_library.py: in progress
- populse_mia/user_interface/pipeline_manager/process_library.py
- populse_mia/user_interface/pipeline_manager/pipeline_editor.py
- populse_mia/user_interface/data_viewer/__init__.py - populse_mia/user_interface/data_viewer/data_viewer.py - populse_mia/user_interface/data_viewer/data_viewer_tab.py
- populse_mia/user_interface/data_viewer/anatomist/__init__.py - populse_mia/user_interface/data_viewer/anatomist/mia_anatomist.py
- populse_mia/user_interface/data_viewer/anatomist_2/__init__.py - populse_mia/user_interface/data_viewer/anatomist_2/mia_anatomist.py - populse_mia/user_interface/data_viewer/anatomist_2/snd_window.py
- populse_mia/user_interface/data_viewer/anatomist_2/anasimpleviewer2.py: in progress
- populse_mia/user_interface/data_viewer/anatomist_2/anasimpleviewer2.py: in progress
- populse_mia/user_interface/data_viewer/anatomist_2/anasimpleviewer2.py: in progress
- populse_mia/user_interface/data_viewer/anatomist_2/anasimpleviewer2.py
We are currently adapting Mia to the latest version of populse_db (V3). I am opening this pull request to merge the dv_v3 branch with the populse_mia master branch. As part of this adaptation, we are taking the opportunity to review and clean up the entire populse_mia codebase thoroughly. Naturally, this will take more time than simply adapting to populse_db V3. We will be regularly adding changes to the populse_mia dv_v3 branch.
The first major change involves the populse_mia.data_manager.database_mia module. This module provides the tools Mia needs to interact with populse_db V3. @sapetnioc , could you please review this new module and share your thoughts, especially regarding execution speed? Are there any suggestions to improve the performance of certain methods within this module?
We will make sure to merge the dv_v3 branch into the populse_mia master branch (this PR) at the same time as merging populsedb3 into the capsul master, 3.0 into the populse_db master, and db_v3 into the mia_processes master.