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.

Sign up for GitHub

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

Jump to bottom

fix(dcd): update to OpinionSiteLinear #1020

Merged
merged 5 commits into from
Jun 11, 2024
Merged

fix(dcd): update to OpinionSiteLinear #1020

merged 5 commits into from
Jun 11, 2024

Conversation

grossir
Copy link
Contributor

@grossir grossir commented May 6, 2024

Solves #1019

Updating base class to OpinionSiteLinear makes code shorter and cleaner, also solves the following bug:

InsanityException caused by unexpected extra line of text in case name cell: "LEAVE OF COURT REQUIRED FOR FILINGS.", which inflated the case_name count by 1

@grossir
fix(dcd): update to OpinionSiteLinear
bcb1af6 
Solves freelawproject#1019

Updating base class to OpinionSiteLinear makes code shorter and cleaner, also solves the following bug:

InsanityException caused by unexpected extra line of text in case name cell: "LEAVE OF COURT REQUIRED FOR FILINGS.", which inflated the case_name count by 1
@grossir
feat(OpinionSiteLinear): add all "optional" field getters
7c9a9fe 
fix(dcd): remove `nature_of_suit`

- Remove nature of suit extraction following discussion on freelawproject#1020
- OpinionSiteLinear supporting all optional fields reduces code duplication, and a getter in `dcd`
@grossir
Copy link
Contributor Author

grossir commented May 9, 2024

So adding support for all optional fields to OpinionSiteLinear breaks the tests.local.test_ScraperExampleTest because fields are collected now, that were silently ignored before, on other sites such as fla with the disposition field. Will make a separate issue / PR to solve that and tag you when this one is ready

@grossir grossir mentioned this pull request May 9, 2024
Open
@grossir
Copy link
Contributor Author

grossir commented May 10, 2024

@flooie please check #1028 and the associated PR , to put the getters on OpinionSiteLinear

@flooie
Copy link
Contributor

flooie commented Jun 7, 2024

@grossir I think this needs to be split into two separate PRs.

The tests are failing because the addition of the optional getters is now causing other court tests to fail because they were failing silently before. Now that you are fixing the silent fail - we should fix all those tests. Those though should be handled in a separate PR. Then lets fix DCD

@quevon24 quevon24 self-requested a review June 11, 2024 19:39
@quevon24
Copy link
Member

@grossir We need to merge this: #1029 before this PR?

@grossir
Copy link
Contributor Author

grossir commented Jun 11, 2024

@quevon24 please check again, I just separated the fix for dcd.

The missing fields will be collected on the other PR

quevon24
quevon24 approved these changes Jun 11, 2024
View reviewed changes
Copy link
Member

@quevon24 quevon24 left a comment

Choose a reason for hiding this comment

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

Great @grossir, It is ready to merge

@quevon24 quevon24 merged commit ce84cbc into freelawproject:main Jun 11, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flooie flooie flooie left review comments

@quevon24 quevon24 quevon24 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
@quevon24's backlog
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@grossir @flooie @quevon24