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

Add support for treaties #121

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

bbernicker
Copy link
Contributor

In response to issue #48 I offered to add support for treaty sources to reporters-db. This PR does so for the treaty sources in Indigo Book T6.

Treaty sources are stored in either the laws.json or reporters.json file depending on which style they more closely resemble. Regardless of where they are stored, they all have cite_type "treaty." The test.py and README.rst files have been updated to recognize "treaty" as a valid cite_type. All examples for all treaties are from Court Listener except for the example for Pan-Am. Treaty Series, which is from a law review article.

I omitted two T6 sources from this PR (Statutes at Large and LEXIS) because they are not exclusive to treaties. Both are already present in reporters-db, and I added a note to the Statutes at Large one explaining that citations there could be to treaties. LEXIS citations are already their own specialty type.

Also, the regex and example for Treaty Series (T.S.) begin with a white space. This is to prevent false positives where T.S. is part of the name of another reporter, but it is an inelegant solution. I tried various lookbehinds and negative assertions, but because they are at the beginning of the string they cause problems.

@mlissner
Copy link
Member

Thanks, @bbernicker. I just put this on @flooie's backlog: https://github.com/orgs/freelawproject/projects/27/views/1. We're a bit behind right now, but we'll be climbing out soon and replying to this ASAP.

@flooie
Copy link
Contributor

flooie commented Sep 2, 2022

@bbernicker not to leave this PR alone. Could you resolve the conflicts, at the very least to get our tests to run.

@bbernicker
Copy link
Contributor Author

bbernicker commented Sep 3, 2022

Yes sorry @flooie . There were no conflicts when I submitted it, but when you approved the PR request for the public/private law citations it created a conflict, which I have now resolved. This should be ready for your review.

reporters_db/data/laws.json Outdated Show resolved Hide resolved
reporters_db/data/laws.json Outdated Show resolved Hide resolved
reporters_db/data/laws.json Outdated Show resolved Hide resolved
reporters_db/data/laws.json Outdated Show resolved Hide resolved
reporters_db/data/laws.json Outdated Show resolved Hide resolved
Copy link
Contributor

@flooie flooie left a comment

Choose a reason for hiding this comment

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

@bbernicker this looks great and pretty close to done. I have a few small things to discuss or talk about but not much.

This responds to @flooie's comments on the last version of the PR. I pulled out the (?:No\\.|Number) group and replaced it with $number_label, which I added to regexes.json. I also added some new examples for treaties using Number instead of No. Finally, I cleaned up some law citations that were not to treaties but which tried to match No. without escaping the period. I replaced them with $number_label.
@bbernicker
Copy link
Contributor Author

bbernicker commented Sep 7, 2022

@bbernicker this looks great and pretty close to done. I have a few small things to discuss or talk about but not much.

Great, thanks @flooie. I made all of the changes you suggested except that I did not change my treatment of mlz_jurisdiction pending further comments on #130.

@bbernicker
Copy link
Contributor Author

This should be ready now for a second review.

@flooie
Copy link
Contributor

flooie commented Sep 13, 2022

I have a handful of other things.

  1. Tests fail when run locally. Treaty needs to be added to the tests.
  2. More concerning is there is a discrepancy being caught by the eyecite tests. For example,

In a random opinion see below, the citation here wraps with two new lines as far as I can tell.

image

This citation when run against the current main returns [FullLawCitation('10 U.S.C. §\n\n859', groups={'title': '10', 'reporter': 'U.S.C.', 'section': '859'}, .....

But in your code, and Im not sure why no longer correctly finds this citation and instead replaces it §§

I rebased your code as well onto the current main and it exhibits the same issue.

@flooie
Copy link
Contributor

flooie commented Sep 13, 2022

@bbernicker Thanks. sorry for the delay on this but I wasn't sure what was going on. But there is certainly something here that isn't working. I suspect we may need to add another test to catch whatever is going on here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Pull Requests
Development

Successfully merging this pull request may close these issues.

3 participants