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

hal: use inspire-utils to parse names #2992

Merged
merged 3 commits into from
Nov 26, 2017
Merged

hal: use inspire-utils to parse names #2992

merged 3 commits into from
Nov 26, 2017

Conversation

jacquerie
Copy link
Contributor

@jacquerie jacquerie commented Nov 25, 2017

Related issues:

Addresses #2273, supersedes #2990.

Checklist:

  • I have all the information that I need (if not, move to RFC and look for it).
  • I linked the related issue(s) in the corresponding commit logs.
  • I wrote good commit log messages.
  • My code follows the code style of this project.
  • I've added any new docs if API/utils methods were added.
  • I have updated the existing documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@ghost ghost assigned jacquerie Nov 25, 2017
@ghost ghost added the Status: WIP label Nov 25, 2017
@jacquerie jacquerie changed the title Remove nameparser dependency hal: use inspire-utils to parse names Nov 25, 2017
@jacquerie
Copy link
Contributor Author

The record that doesn't pass validation in https://travis-ci.org/inspirehep/inspire-next/jobs/307298427 is because of inspirehep/inspire-schemas#280.

@jacquerie
Copy link
Contributor Author

And all the failing tests in https://travis-ci.org/inspirehep/inspire-next/jobs/307298424 might be because of the same reason, as pdf_url_correct gets put in urls.value, but it isn't a valid URI.

@@ -72,7 +75,8 @@ def test_set_schema_builds_a_full_url_for_the_schema():

data = {'$schema': 'hep.json'}
extra_data = {}
assert validate(data['$schema'], subschema) is None
with pytest.raises(ValidationError):
validate(data['$schema'], subschema) is None
Copy link
Contributor Author

@jacquerie jacquerie Nov 26, 2017

Choose a reason for hiding this comment

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

This is notable: inspire-dojson puts something invalid there, and set_schema makes it valid. This is the only exception to the usual task invariant (valid in, valid out).

As currently the Authors schema can't correctly express a Twitter
profile link we remove this field so that it won't cause a validation
error during migration of the demo records.

This commit should be reverted when inspirehep/inspire-schemas#182
is merged.

Signed-off-by: Jacopo Notarstefano <[email protected]>
Signed-off-by: Jacopo Notarstefano <[email protected]>
Signed-off-by: Jacopo Notarstefano <[email protected]>
@jacquerie jacquerie merged commit c6e5011 into inspirehep:master Nov 26, 2017
@jacquerie jacquerie deleted the remove-nameparser-dependency branch November 26, 2017 14:35
@ghost ghost removed the Status: WIP label Nov 26, 2017
@jacquerie jacquerie removed their assignment Dec 13, 2017
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.

1 participant