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

JSON metadata -- set language #1211

Closed
wants to merge 9 commits into from
Closed

JSON metadata -- set language #1211

wants to merge 9 commits into from

Conversation

charlie-jones
Copy link
Contributor

Description of proposed changes

I mainly followed the ideas from this comment on the issue (#1049 (comment))
I changed code in the general reducer and in recomputeReduxState. For the general reducer, I modeled everything after the controls reducer. (so the same format for getting displayDefaults metadata. The code for that is in recomputeReduxState)

Related issue(s)

Here: #1049

Testing

However, I'm sending untested code because I don't know how to run the test JSON. (https://github.com/nextstrain/auspice/blob/master/examples/minimal_v2.json#L37).

Hope this helps! And, if I could get help on how to run the test JSON, I can work on it more next weekend.

Copy link
Contributor

@eharkins eharkins left a comment

Choose a reason for hiding this comment

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

Hi @charlie-jones, thanks for your work on this again! It seems like you are generally on the right track. Here's how you can get setup to run auspice and test:

  1. Install auspice locally: https://nextstrain.github.io/auspice/introduction/install#installing-from-source
  2. run auspice with the example dataset directory auspice develop --datasetDir examples (https://nextstrain.github.io/auspice/introduction/how-to-run#how-to-get-an-example-dataset-up-and-running)
  3. It would also be helpful (for you as you develop and for us when we merge) to run npm run lint to format the code. This will require having installed as above. You can also see where linting checks failed here.

In terms of the implementation, I realize now that setting the display component (indirectly by getting the default state here) as a result of loading a JSON might get you stuck in a loop if it defaults to the datasetLoader which then calls loadJSONs again. Taking a step back from this, it seems like the general piece of the state should be treated differently from the metadata
piece in that we should probably not start with a totally clean slate in this case. I can think of two ways to treat the general state specially and just set the defaults, and the language properties (and any other display defaults that we may track in the general state in the future):

  1. Still use the CLEAN_START action as you have done, but in the reducer for the general state, only replace certain parts of the state when handling that action
  2. Dispatch a different action just for the general state, in addition to the CLEAN_START one which gets dispatched when the json is first loaded

Hope this is helpful, I can try to be more specific if not. Appreciate you working on this!

@charlie-jones
Copy link
Contributor Author

Hi @eharkins, really appreciate the feedback! Thank you! I went with implementing #1. When running it on the main datasets (npm run view/dev), I noticed that the language field is empty, and there were on/off problems with changing the language through querying. On the minimal_v2 json with datasetDir and it runs smoothly and works. Any ideas about this? (anyone). I can work on this more, and I'd appreciate guidance. Thanks!

@charlie-jones
Copy link
Contributor Author

Hi all, I'm glad to say that I made some updates today and the language from json works now! On npm run view, npm run dev -- it functions normally, and does not use json data (because that's in augur?). On node.js auspice --view --datasetDir examples, the program reads from the JSON (currently set to Japanese). An important note - the program seems to need this CreateMetadataFromJSON import in the loadData class. During testing, without that import, the program only worked on "npm run dev" and running with --datasetDir examples. Hope this helped. Thanks!

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