Materials-Consortia / optimade-tutorial-exercises

Tutorial exercises for the OPTIMADE API
https://optimade.org
MIT License
15 stars 7 forks source link

Add JavaScript OPTIMADE module example #13

Closed blokhin closed 2 years ago

blokhin commented 2 years ago

@JPBergsma which Node version do you have? SyntaxError: Unexpected token signals it's outdated.

JPBergsma commented 2 years ago

I have nodejs version v10.19.0 This seems to be the default for Linux Mint 19.3 and Ubuntu 18.04

blokhin commented 2 years ago

The problem with this stack is it's getting outdated very quickly. The distros default versions are usually too old. I added the requirement of node -v >= 14 and npm -v >= 7.

blokhin commented 2 years ago

PS managing Node JS (and hence NPM) versions is convenient with https://github.com/nvm-sh/nvm

JPBergsma commented 2 years ago

I have now installed nodeJs v17.2.0 which comes with npm version 8.1.4 and executed the steps in Readme.md It however gives the error message: "http-server: command not found". This is probably caused by the missing dependency isomorphic-unfetch

Output of the script:

npm install
npm WARN deprecated urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
npm WARN deprecated resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated
npm WARN deprecated sane@4.1.0: some dependency vulnerabilities fixed, support for node < 10 dropped, and newer ECMAScript syntax/features added
npm WARN deprecated tslint@6.1.3: TSLint has been deprecated in favor of ESLint. Please see https://github.com/palantir/tslint/issues/4534 for more information.

added 550 packages, and audited 551 packages in 5s

27 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

npm run build

> optimade@2.0.4 build
> npm run lint && rollup --config

> optimade@2.0.4 lint
> tslint -c tslint.json "src/**/*.ts"

src/index.ts:34:23
WARNING: 34:23  no-shadowed-variable  Shadowed name: 'api'
WARNING: 45:30  no-empty              block is empty
WARNING: 88:35  no-shadowed-variable  Shadowed name: 'structures'
WARNING: 89:13  no-console            Calls to 'console.dir' are not allowed.
WARNING: 101:1  max-line-length       Exceeds maximum line length of 180

src/index.ts → dist/index.mjs, dist/index.js...
(!) Unresolved dependencies
https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency
isomorphic-unfetch (imported by src/utils.ts)
created dist/index.mjs, dist/index.js in 1.2s

npm install http-server

added 27 packages, and audited 578 packages in 3s

34 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
blokhin commented 2 years ago

@JPBergsma isomorphic-unfetch has nothing to do with http-server... But what happens if you do npm install isomorphic-unfetch?

JPBergsma commented 2 years ago

I have executed npm install isomorphic-unfetch. I did not receive an error message while doing this, but the number of audited packages remains the same. http-server is still not found.

blokhin commented 2 years ago

@JPBergsma could you do npm install -g http-server i.e. install a server globally? Then the command http-server should work.

JPBergsma commented 2 years ago

After npm install -g http-server I no longer get the error "http-server: command not found".

In the future also people who have not followed your lecture may also want to do the OPTIMADE tutorial. So, I think it would be good to give a bit more of an explanation about what we are trying to do here.

When I open http://localhost:8080/examples/search.html I get a blank page. This seems a bit strange to me.

blokhin commented 2 years ago

When I open http://localhost:8080/examples/search.html I get a blank page. This seems a bit strange to me.

A console should be opened (CTRL+SHIFT+I), this is mentioned in the README.md. In general, compiling the code is optional. There is just one example (as the name of the folder suggests): mpds_gifs.html. It works without the code compilation. If a reader has no idea what to do with this example, he should not do this tutorial.

ml-evs commented 2 years ago

Hi @blokhin, does this accompany your workshop talk at the EAIFR? I wonder if you could structure it more like a tutorial exercise to match the others in this repo - these have been targeted at students so far, but this looks more targeted towards developers, right?

blokhin commented 2 years ago

@ml-evs sorry for the delays with answers, very hectic times... Yes, this accompanied the workshop, but the overall message is a bit different. This is more for the curious users, not necessarily for the developers. That is why I decided to create the separate folder. Eventually it's very orthogonal to the other exercises, it's another technology and another way of accessing the data. Honestly it deserves its own repository, but I don't have time to create it :disappointed: I realize only a few people will read it as it is now, but may be exactly they will create completely new things based on the Optimade. The MPDS was taken just as an example, the optimade library will of course work with any other provider.

blokhin commented 2 years ago

@ml-evs any chance to accept it before the new year?

blokhin commented 2 years ago

@ml-evs ping

blokhin commented 2 years ago

@gmrigna dear Gian-Marco, could this be merged?

ml-evs commented 2 years ago

Hi @blokhin, sorry, swamped with GitHub notifications at the moment.

Have you addressed the comments from @JPBergsma? I'm still not sure this tutorial is appropriate for this repo really (though I understand your answer above).

Could you perhaps move the javascript-example folder out of the top level and to the notebooks directory? I can also add a link to it from the main exercises notebook.

blokhin commented 2 years ago

@gmrigna I find very strange there were so many obstacles accepting my contribution.

ml-evs commented 2 years ago

@gmrigna I find very strange there were so many obstacles accepting my contribution.

Hi @blokhin, a lot of effort went into making sure the other tutorials worked robustly (see the edits of the requirements, patches submitted to pymatgen etc.), this was partially due to feedback from participants in the workshops we ran, who needed to be able to run these tutorials without our help. I am happy to include the javascript tutorials in this repo (it is not up to me to accept/deny, I am just the one who had to write the other tutorials...), but I think @JPBergsma's comments should be addressed to make sure the code works before merging.

Moving the tutorial to a subdirectory does not seem like a big ask, it just allows the reference to all the tutorials in the notebooks directory to also apply to your exercise (which I can add in the same section as the Example Python code on the homepage, so it is not hidden away).

I do not have the time to fix your tutorial myself, at the moment. If you would rather, you should have equal admin rights in this organization as myself to make a new repo for the JS tutorials.

gmrigna commented 2 years ago

Hi @blokhin Let me first thank you very much for your effort and all the time that you have put and are putting into this. I am sure that we will manage to sort this out and that your contribution will be really appreciated by the people going through the tutorial in the future... I must say that @ml-evs points make sense to me especially because we are not really in a hurry now (our next tutorial is scheduled in June). Or am I missing something? Don't you think that it is probably better to take the time to solve all the problems before merging. Also, the period was a bit particular (winter holiday) which has probably slowed things down. All our apologies about that. I would like to mention that both @ml-evs and @JPBergsma have a number of other things which require all their attention at the moment (like finishing the writing of a thesis and advancing the project on which they are paid because the renewal of their position depends on it). So, I would like to kindly ask you to be patient here. As I said, we will sort this out...