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

Patch to custom env docs #197

Merged
merged 5 commits into from
Jun 6, 2023
Merged

Patch to custom env docs #197

merged 5 commits into from
Jun 6, 2023

Conversation

emileten
Copy link
Contributor

@emileten emileten commented May 18, 2023

It's not very clear where to find the example environment configuration files when reading the custom environment docs.

Just added an additional pointer that explicitly says the config files are in the repo containing the source code of the docs.

There is probably a better way but this is just a quick improvement.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@wildintellect
Copy link
Collaborator

Any reason to not just link to the github location?

@rtapella
Copy link
Collaborator

FYI I have merged getting-started into develop. Can we re-point this PR to develop?

@emileten
Copy link
Contributor Author

emileten commented May 18, 2023

Sure will move this.

@wildintellect which branch would I point to ? what if users are looking at the version of another branch?

@wildintellect
Copy link
Collaborator

@emileten can you provide some urls, so we can examine if we should always point to master or perhaps the files are in the rendered site and a relative url works?

@emileten emileten changed the base branch from Getting-Started-pr-branch to develop May 19, 2023 00:21
@emileten
Copy link
Contributor Author

@wildintellect the files aren't in the rendered site. That could be a todo for later, would be more user friendly.

The file sources is here https://github.com/MAAP-Project/maap-documentation/tree/develop/docs/source/system_reference_guide/example_conda_configuration_files.

I don't think these will be change often, so it should be ok to just always point to that link?

@wildintellect
Copy link
Collaborator

@emileten this raises a question, should those file be buried deep in the website source, or should they be brought up much higher in the directory structure for easier access when cloning the repo? Example https://github.com/aws/studio-lab-examples/tree/main/custom-environments puts a folder at the repo top level. This way even if we re-organize the pages later these files won't move.

Also wondering if we should give them more descriptive env names? and if we should make some real useful examples rather than just examples for examples sake?

@emileten
Copy link
Contributor Author

emileten commented May 23, 2023

@emileten this raises a question [...]

Suggestion @wildintellect : I add a notebook that looks like this and I link that in the custom-env explanation notebook. That way the yaml files referenced are always the right ones.

Also wondering if we should give them more descriptive env names?

Agreed, I will address this in this PR

And if we should make some real useful examples rather than just examples for examples sake?

Agreed but that takes more time so we should do this in a later issue/PR

@wildintellect
Copy link
Collaborator

Sure we can do better examples later. A few comments:

  • At a minimum these should always include ipykernel
  • For later, we need a R example with an Rkernel
  • does the base.yml conflict with the base env already in a workspace?

@emileten
Copy link
Contributor Author

emileten commented May 23, 2023

At a minimum these should always include ipykernel
For later, we need a R example with an Rkernel

Adding these to an 'update examples' ticket along with 'real useful examples rather than just examples for examples sake'

does the base.yml conflict with the base env already in a workspace?

I don't think it does since I remember testing all of these configs. To be clear, it's a config used to show how to update an environment. Should rename it to update_base.yml.

#204

@emileten
Copy link
Contributor Author

emileten commented May 24, 2023

@wildintellect

Now I am pointing to a notebook showing the example config files. The notebook lives in the same section (system reference guide). Also renamed the files so that the names align with the example purpose.

Here is part of the notebook in a screenshot. It's admittedly not perfect, e.g. one has to copy paste what's printed and put it in a yaml file. But I think it's better than before ?

I'd like to hide all the code but I could not figure how to do that.

Screenshot 2023-05-24 at 4 01 56 PM

@rtapella
Copy link
Collaborator

I think listing the contents like this is fine. It seems most likely that it'll be used as a template, anyway, and someone would copy/paste from a file or from the notebook and then edit it.

Potentially we could suggest that the workspaces have these environment examples built-in, in a standardized location. That way everyone would start to put their env files in the same location and have some examples to start with.

@emileten
Copy link
Contributor Author

emileten commented Jun 1, 2023

@rtapella I quickly tried after your suggestion https://github.com/mamba-org/gator#Navigator and it both works relatively smoothly and is nice. It also looks acceptably active.

So if we follow your suggestion

Potentially we could suggest that the workspaces have these environment examples built-in, in a standardized location. That way everyone would start to put their env files in the same location and have some examples to start with.

We could replace the pointer to this printer notebook with a pointer to the environments UI.

My question is : if we want to do that, should we hold on merging this ? Or should we merge this, and then figure out the UI addition ?

This is ready by the way. Below a screen of what the index looks like.

Screenshot 2023-06-01 at 9 43 26 AM

@wildintellect
Copy link
Collaborator

@rtapella yes having the whole docs installed by default has been mentioned several times (future scope)
@emileten the visual tool does not solve this issue, we need users to make yaml files because they will need to supply those for DPS algorithms. It's just an extra, I consider out of scope for this version.

@emileten emileten merged commit db6df64 into develop Jun 6, 2023
@wildintellect wildintellect deleted the custom-env-docs-patch branch July 20, 2023 20:13
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