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

Changes of joacmue #14

Open
wants to merge 139 commits into
base: master
Choose a base branch
from
Open

Changes of joacmue #14

wants to merge 139 commits into from

Conversation

ulfgebhardt
Copy link
Member

Hey @joacmue

would you be interested in getting your changes in the master?
Would that be applicable?

Interested in becoming a maintainer?

@joacmue
Copy link

joacmue commented Mar 28, 2021

Hi @ulfgebhardt,
sure, merging that back into your fork would totally work for me. It just seemed pretty dead over the past months ;)
I'm not that versed around open source development, so... should I start a PR from my fork to yours?

Not sure what I'm getting myself into by becoming a maintainer, but I'd be open to contribute.

@joacmue
Copy link

joacmue commented Mar 28, 2021

Looking over the other current activities, this one mainly fixes lawdown and adds some image/table features to it (my test case was the StVO which is pretty heavy on images in tables for Traffic Signs and stuff). Might only want to review the changes to lawdown and readme.md and just throw away the other changes.
I tried going into the banz_scraper but ran into issues with their page redesign pretty fast. If I recall correctly, I mainly noted down some new candidate links, but stuff seems to be better addressed by #18

@ulfgebhardt
Copy link
Member Author

Hey @joacmue nice to hear from you. I believe I could incite some activity into the project lately. I was informed that you made some nice changes not to long ago and just wanted to expose those to this repo.

Would you be interested i contributing? I believe we are on a good way to get things running and have an up to date https://github.com/bundestag/gesetze repo soon. With your help sooner and better <3

With the looming Election this year having the repo up to date would save me from millions of questions why its not up to date ;)

@joacmue
Copy link

joacmue commented Mar 28, 2021

So, I just rebased my fork to what happened here but did not get the whole pathlib stuff right away, so most of the changes made there might have gotten lost. Feel free to add them back during the review.

@joacmue
Copy link

joacmue commented Mar 28, 2021

So I just recognized the bigger picture of pushing the actual laws to https://github.com/bundestag/gesetze That puts the work here in a new perspective for me. please stand by while I process this.
Kidding aside, once my slightly more involved state stuff in the parser got merged back, I'll see where I feel comfortable helping out on issues. Might need some help with the actual scrapers, though.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
bgbl_scraper.py Outdated Show resolved Hide resolved
@ulfgebhardt
Copy link
Member Author

I believe we have to copy the PR into the repo or rebase in oder for the tests to run. I cannot accept your suggestions. I think only @joacmue can, since hes the owner of the fork.

@darkdragon-001
Copy link
Collaborator

@ulfgebhardt We should give @joacmue some time to work on this...

darkdragon-001 and others added 23 commits May 15, 2021 13:50
- Fixed string types
- Fixed bug where table rows started with a space character
- Fixed bug where new folders where not created multi-level
- Fixed unimplemented option --no-yaml
Putting the content somehow in the middle
Strangely, round(0.5) yields 0 but round(1.5) yields 2?
Replaced with floor and ceil, importing them from math
Had a typo in parsing for the thead state -.-
Added it to a check in the end
There is room for improvement on rendering nested tables, I'm sure
Actually keping track of which column has entries now
thereby printing later additions below earlier ones even for colspans
Had to run bgbl_scraper in between - updated that in readme.md
@joacmue
Copy link

joacmue commented May 15, 2021

So, I rebased to the master from here. lawdown.py still works on everything wth the expected results, so I guess I made that right.

However, I noticed that the banz_scraper fails with an error. I copied over the code from master here and I do have the same error, so I guess there's some fault in the code there. Not gonna put cleaning that up in this PR, though ;)

@darkdragon-001
Copy link
Collaborator

Tables look good now 👍 Thanks for your great work so far!

I resolved all comments which are good now and commented on the remaining ones. Especially the regression mentioned in #14 (comment) should be resolved before merging IMHO.

@ulfgebhardt
Copy link
Member Author

Can't we make Issues for the remainign Issues and merge? This PR is way to big already

@4350pChris
Copy link

Any update on this?

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.

None yet

4 participants