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

Fix self.line typos in call to BadSyntax. #1529

Merged
merged 2 commits into from Dec 28, 2021
Merged

Fix self.line typos in call to BadSyntax. #1529

merged 2 commits into from Dec 28, 2021

Conversation

ghost
Copy link

@ghost ghost commented Dec 24, 2021

Fixes #821

Proposed Changes

  • Change incorrect references from self.line to self.lines in 3 calls to BadSyntax in notation3 parser.

Fix for issue #821 Invalid URI crashes without BadSyntax error
@nicholascar nicholascar self-requested a review December 28, 2021 11:26
Copy link
Member

@nicholascar nicholascar left a comment

Choose a reason for hiding this comment

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

Approving and merging as a simple PR (but good to fix the bug!)

@nicholascar nicholascar merged commit 26ad286 into RDFLib:master Dec 28, 2021
@ghost ghost deleted the fix-issue821 branch December 28, 2021 12:24
@ghost
Copy link
Author

ghost commented Dec 28, 2021

Approving and merging as a simple PR (but good to fix the bug!)

There's a few similar small-bugfix PRs stacked up, I bravely resisted the temptation to bulk 'em up into a single PR, so sorry for the extra work on your side.

@nicholascar
Copy link
Member

Definitely the right thing to do to keep ‘em all small! Happy to do the extra resulting reviews.

Really glad you are grinding through all these as these plus all the testing and environment improvements - top etc. - means the codebase is really neatening up. I’m really happy about this as I’d like to see some some major changes for a 7.0.0 early next year but ideally they should be implemented on top of tidy code rather than having to tidy code within their major changes.

The major changes are:

  • your work on Graph ID
  • ConjunctiveGraph / Dataset dedup
  • RDF-Star
  • JSON-LD 1.1 completion
  • Store comparison / alignment
  • SPARQLWrapper something or other (fix / retire / integrate / something)

@ghost
Copy link
Author

ghost commented Dec 28, 2021

Definitely the right thing to do to keep ‘em all small! Happy to do the extra resulting reviews.

Good to know, trying to stay out of your way.

Really glad you are grinding through all these as these plus all the testing and environment improvements - top etc. - means the codebase is really neatening up.

To provide a modicum of context - at 70, I'm officially a pensioner but "semi-retired" in that I've still got some skin in the game - I use RDFLib as a back-end for ACME (A Cryptocurrency Metadata Explorer), so I'm quite happy to have something to occupy me other than daytime TV, even better that its purposeful.

I’m really happy about this as I’d like to see some some major changes for a 7.0.0 early next year but ideally they should be implemented on top of tidy code rather than having to tidy code within their major changes.

As I've observed in the Discussion welcome, 2022 is RDFLib's 20th anniversary. Would be nice to see a neat codebase with a clear set of maintenance tasks and decent documentation. It's fortuitous that Github has integrated Discussions/Issues in such as way as to add a back-and-forth path, that really helps with migrating not-actually-issues out of the issues list and clarifying the remaining issues.

* your work on Graph ID
* ConjunctiveGraph / Dataset dedup

I'm not familiar with the dedup intentions and I really should be, is there a pointer? gromgull summarised his understanding of the two in #307, I'm pretty much working to that model.

* Store comparison / alignment

I've made some reasonable progress with an ansible/vagrant script for this purpose, it's on the back-burner for the time being.

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.

Invalid URI crashes without BadSyntax error
1 participant