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

please review/test new tutorials for oepclient #206

Closed
wingechr opened this issue Jul 14, 2023 · 4 comments
Closed

please review/test new tutorials for oepclient #206

wingechr opened this issue Jul 14, 2023 · 4 comments
Assignees

Comments

@christian-rli
Copy link
Contributor

Thank you for creating these @wingechr

Everything worked for me up to 04_client_cli_upload! notes:

01_client_python_download.ipynb

  • it took me a minute to figure out that the example table is located on the platform. everything worked so quickly. that's a good thing. but i think we should mention somewhere that the table is located in model_draft or even better provide a link to it

02_client_python_upload.ipynb

  • nifty randint trick!
  • how would you like to address the TODOs/explanations? Suggestion: If you give me just raw and rough content, I can turn it into explanatory but succint sentences... and I would also like to know the link to where all data types are explained (or is it just the postgres documentation, and if so, do we support all types?)
  • We really should link to the created tables on the OEP. That's a crucial point of information.

03_client_cli_download.md

  • I just learned from you that you can just set local variables with '=' andyou don't have to set a system variable with 'export'. Very handy.
  • As a general remark: We could opt for a separation by operating system rather than topic, which may look a little bit more tidy. I'm not sure it would have this effect though and it's not a strongly held opinion.

04_client_cli_upload.md

  • I stumbled across some pitfalls here
  • As I don't have an environment variable set for the OEP token, the SETUP commands executed without an error, but left me without a working setup. I think there should be a note that you need to have the API token set as environment variable OR that you need to paste it into the code OR you should be prompted for it. Which do you think is better? In any case I think the command should be separated out, as it is a likely pitfall and needs at least some explanation.
  • I was able to run everything eventually, but again, i think it's necessary to link to the created table and metadata, because otherwise it seems like a dry exercise and people could miss that they're actually interacting with a server here.

@wingechr
Copy link
Contributor Author

@christian-rli thanks for the testing

  • about the TODOs/explanations: I just don't know how much to explain here. It should be as short as possible, but should explain the required data structure of the table schema and possible options.

  • environment variable: I guess we need to write a sentence explaining: Either you have already an environmental variable OEP_API_TOKEN, or you have to assign your token in the commandline to $token

@flohrreija-oeko
Copy link
Collaborator

flohrreija-oeko commented Jul 16, 2024

Some general comments on all oepclient tutorials:

  • I would suggest to the add the feedback link to this issue both at the beginning and in the about section of the tutorial (
  • The code did not work (understandably) until i was a member of the OEP group in GitHub, it could be added as a requirement for clarity

Comments on 01_client_python_download.ipynb

  • It also (see first comment of the issue) took me some time to understand that "tutorial_example_table" is an already existing table in model_draft - either a hint on this in the comments of the code or e.g. a codeline to get the url where the table is located (also to verify if everything worked) would be great!

Comments on 02_client_python_upload.ipynb

  • Here as well (again as first comment) I think it is super important to be able to access the URL of the created table to verify while doing the tutorial.
  • If I understand correctly in Section "upload data" we use an existing dataset in the OEP (in this case "tutorial_example_table) to upload data into the table we just created - normally the data to be uploaded would not come from an existing dataset in the OEP but from e.g. an Excel or textfile. I think it should be clarified how to upload the data from there as well using the OEPClient (either through linking to section 2.1.2 of the Beginners Guide or by including a similar example code in the OEP Client tutorial 02).

@han-f han-f assigned wingechr and unassigned christian-rli and jh-RLI Jul 17, 2024
@han-f
Copy link
Contributor

han-f commented Jul 17, 2024

Closing this one, as we now have single issues per tutorial:
#235 for oeclient tutorial 01
#236 for oeclient tutorial 02
#237 for eclient tutorial 03
#238 for oeclient tutorial 04

@han-f han-f closed this as completed Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants