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

Add openalea namespace #122

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add openalea namespace #122

wants to merge 3 commits into from

Conversation

thomasarsouze
Copy link
Contributor

No description provided.

@pradal
Copy link
Contributor

pradal commented Jan 22, 2024

@thomasarsouze : if you modify the namespace, you will have to change all the tests, the examples, and the doc also.

@RamiALBASHA RamiALBASHA self-requested a review January 22, 2024 21:17
@RamiALBASHA RamiALBASHA self-assigned this Jan 22, 2024
@thomasarsouze
Copy link
Contributor Author

@pradal : yes this is what I thought, but as mentioned yesterday, we're working first on packages that already have the namespace, which is not the case here.
Also, this is good to first discuss and confirm all that with @RamiALBASHA : I you think this is ok with you, just let me know, I can do the job and let you do the review to check everything. Just to be very clear, we're talking here about having the same way to name all openalea packages for the upcoming release, which would mean in this case switching from import hydroshoot to import openalea.hydroshoot.

@RamiALBASHA
Copy link
Collaborator

@thomasarsouze I think that this is a good idea to harmonize the way openalea packages are treated, although it makes things a bit verbose.
I think however that we should make a minor increase to the version. By doing so, I suppose that we will prevent future errors from raising for codes already calling hydroshoot with an explicit version number (prior to the one with the new namespace). Do you confirm?
Best

@thomasarsouze
Copy link
Contributor Author

Sorry for the late reply @RamiALBASHA. Indeed it is much safer to have a new release associated with this namespace change to prevent code break or at least make it easier to debug. As the hydroshoot package manager, I let you decide how to tag your version.

Copy link
Contributor Author

@thomasarsouze thomasarsouze left a comment

Choose a reason for hiding this comment

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

Tests all pass ok. However, before merging, please be aware that at the moment, Caribu with corrected namespace has not been released on all OS / python versions (cf. here).

Issue is currently handled though: openalea-incubator/caribu#50

@RamiALBASHA
Copy link
Collaborator

Ok, thanks @thomasarsouze. Is there any deadline for merging? I need to take my time on this issue.

@thomasarsouze
Copy link
Contributor Author

No rush...
It would be nice if hydroshoot can be part of the final openalea release, but you still have some time ahead.

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.

3 participants