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

hep_schema: known categories are now enums #23

Merged
merged 3 commits into from
Oct 14, 2016

Conversation

david-caro
Copy link
Contributor

  • ARXIV and INSPIRE scheme categories have now enums with valid
    options.
  • Other scheme types (APS and POS) can have any values, but no other
    schemes are allowed.

Signed-off-by: David Caro david@dcaro.es

@david-caro
Copy link
Contributor Author

Waiting to resolve current logging issues on record migration on nightly so we will have insight on what this breaks on the current records there.

@david-caro
Copy link
Contributor Author

Actually, we can merge this, as we are using semantic versioning, this will not get deployed on nightly until we change the requirements on inspire-next :)

"enum": [
"INSPIRE",
"submitter",
"conference",
"curator",
"publisher",
"arxiv",
"pos",
"asp",
Copy link
Contributor

Choose a reason for hiding this comment

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

What are pos and asp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PoS = Proceedings of science
ASP is a typo, it should be aps -> American Physical Society

Essentially, two spiders that we already have that use this.

"properties": {
"scheme": {
"type": "string",
"enum": ["INSPIRE"]
Copy link
Contributor

@jacquerie jacquerie Oct 12, 2016

Choose a reason for hiding this comment

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

Mh, maybe we should add something like prettifyjson to this repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't care much, what would it improve?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found an easy way to pretiffy all json, sent a new patch with a script for it + pretified everything with it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... I'll add a test for it too so we don't forget pretifying at the end, maybe even a hook-like script to add to the git before commit or something might work too...

@@ -1,182 +1,172 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I misunderstood how you meant to use your generate_records script, but here IMO it would be better to only have the changes about the thing that you really changed, rather than a completely new record...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, these examples are meant to be to ensure backwards compatibility, so you should not use them to see what has changed. To see the changes you should look to the schema itself.

I agree though that it would be a plus to have the scripts regenerate only the portions that changed, but I see no easy way to do it (that does not involve some non-trivial programming on the ugly js script).

For now I just regenerate the whole lot each time to make sure it's maintained complete.

* INCOMPATIBLE ARXIV and INSPIRE scheme categories have now enums
  with valid options.
* Other scheme types (APS and POS) can have any values, but no other
  schemes are allowed.
* Updated tests fixtures to adapt to the new schema changes.
* Fixed issue on the example generator script to allow intra-schema
  references.

Signed-off-by: David Caro <david@dcaro.es>
* Also pretified all the existings jsons.

Signed-off-by: David Caro <david@dcaro.es>
Signed-off-by: David Caro <david@dcaro.es>
@david-caro
Copy link
Contributor Author

Let's merge this, it will not break anything on inspire-next, but allows testing the related changes on hepcrawl.

@david-caro david-caro merged commit b2e2232 into inspirehep:master Oct 14, 2016
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