Skip to content

Implementation of #402 +species informations - #404

Merged
lesquoyb merged 9 commits into
gama-platform:2024-06from
LOFT-Openlab:improve-402-gama-server-model-description
Feb 14, 2025
Merged

Implementation of #402 +species informations#404
lesquoyb merged 9 commits into
gama-platform:2024-06from
LOFT-Openlab:improve-402-gama-server-model-description

Conversation

@Jarakoto

Copy link
Copy Markdown
Contributor

Add a "describe" gama server command to get a model description which gives :

  • The species names and related actions
  • The experiments and related parameters

Add a "describe" gama server command to get a model description which gives :
- The species names and related actions
- The experiments and related parameters
codescene-delta-analysis[bot]

This comment was marked as duplicate.

- fix command error message
- change key name of action commands to "parameters"
codescene-delta-analysis[bot]

This comment was marked as resolved.

Removed the model in the species list
codescene-delta-analysis[bot]

This comment was marked as resolved.

codescene-delta-analysis[bot]

This comment was marked as duplicate.

@Jarakoto Jarakoto changed the title Implementation of #401+species informations Implementation of #402 +species informations Jan 31, 2025
codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@Jarakoto

Copy link
Copy Markdown
Contributor Author

I would like to divide the nested for loops to the related interface, but it will be used only in the gama server part.

@lesquoyb

lesquoyb commented Feb 10, 2025

Copy link
Copy Markdown
Contributor

Hello and thank you for your contribution!
So far the code seems good, but before merging I think we should pay attention to those points:

  • Check that the command works when gama has a GUI
  • Check that the command works when gama is in headless server mode
  • Add parameters to only get the information that we care about (species, actions, parameters etc.) instead of everything
  • Add a description of the command on the online documentation:
    • general description of what it does
    • parameters needed and optional to call it
    • format of the output
    • possible errors
  • If possible try to comply with codescene quality gates (I haven't checked it yet, sometimes there is just no proper way to do it) => has been improved enough, the rest to do has been identified and tranfered to Refactor, polish and document gama-server and its API #1
  • @AlexisDrogoul This command as well as the load command (and maybe other ones ?) are compiling the model, do you think there's a "clean" way to store the compiled model to avoid possible recompilation that could be long and resource intensive ? =>won't be done

- splits describe into subfunctions
- adds parameters for more granularity
- enforces stronger typing when possible
- fixes wrong parameter outputs for actions
- add the return type of actions
@lesquoyb

lesquoyb commented Feb 10, 2025

Copy link
Copy Markdown
Contributor

Hello @Jarakoto, I have a commit to fix a few minor issues with this PR but I cannot push it because (I think) it is hosted on your github and you have to allow external contribution, here is an example pull request with the same problem:
image
It's the checkbox at the bottom right corner: "allow edits by maintainers"

@Jarakoto

Copy link
Copy Markdown
Contributor Author

Hello @lesquoyb,
Thank you for the the review and the fixes. I wasn't able to see the fix (the PR panel was empty so this checkbox was not displayed). I added you as maintainer of this repo, are you able to push on it ?

uses getOrDefault instead of ternary conditions for parameters
@lesquoyb

lesquoyb commented Feb 12, 2025

Copy link
Copy Markdown
Contributor

So I thought a bit about compiling only once, but finally I don't think it's a good idea as the describe command is independent from loading or not the model, and technically it could be "normal" in certain cases for the model file to be updated in between calls. So for now let's forget about it.

Regarding passing the quality gates of codescene, the only thing left would be to refactore the code for compilation (but not cache it) between describe and load and we should pass it. That being said I prefer to add that task to the issue dedicated to refactoring gama-server in general (#1), as it may be a little bit tricky and out of the scope of this PR.

I've just created a draft of documentation for the new command that you can see here and if you think it's clear enough and I didn't miss anything then we can merge that PR.

@Jarakoto

Copy link
Copy Markdown
Contributor Author

@lesquoyb Thanks a lot for the improvement of the function. It's all good for me.
I would refine the last sentence of the documentation to : "If certain information is requested to be ignored via a parameter (as explained above), the corresponding field(s) will be omitted from the response."

@lesquoyb

Copy link
Copy Markdown
Contributor

Sounds good to me

@lesquoyb
lesquoyb merged commit a6f7855 into gama-platform:2024-06 Feb 14, 2025
@lesquoyb
lesquoyb deleted the improve-402-gama-server-model-description branch February 14, 2025 03:11
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.

2 participants