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

Fixed a bug in the methods to delete name, given name and last name of instances of the ResponsibleAgent class. #16

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

arcangelo7
Copy link
Collaborator

I found and fixed a small bug in the remove_name, remove_given_name and remove_family_name methods of the ResponsibleAgent class. I'll report the code with and without the bug regarding remove_family_name, but the same goes for the other two functions:

# Code with the bug: self.g is not a subject
def remove_family_name(self) -> None:
    self.g.remove((self.g, GraphEntity.iri_family_name, None))

# Code without the bug: self.g -> self.res
def remove_family_name(self) -> None:
    self.g.remove((self.res, GraphEntity.iri_family_name, None))

…f instances of the ResponsibleAgent class.
@iosonopersia
Copy link
Collaborator

Hi @arcangelo7 , thank you for this submission.
I'm going to merge your fix right now.
Are you aware of other bugs that could be solved before publishing a new version of the library?

@iosonopersia iosonopersia merged commit 61e5e4e into opencitations:master Jun 8, 2021
@arcangelo7
Copy link
Collaborator Author

Hi @arcangelo7 , thank you for this submission.
I'm going to merge your fix right now.
Are you aware of other bugs that could be solved before publishing a new version of the library?

Yes, I am aware of another bug, but I only have a temporary solution to it, unconvincing if I have to be honest.
The bug occurs by running the upload_all function of the Storer class and occurs when the update_query returned from get_update_query is None instead of "". In that case if update_query == "" does not work and the function crashes when trying to concatenate query_string += " ; " + update_query at row 279, because it cannot concatenate a string with None.

query_string += " ; " + update_query

My temporary solution was to check whether update_query is None as well as "", this way:

if update_query == "" or update_query is None:
    skipped_queries += 1

However, I do not consider it a solution, because it should be understood why sometimes the update_query is None instead of "".

@iosonopersia
Copy link
Collaborator

get_update_query() returns None when the given entity is to_be_deleted and its preexisting_graph is empty (either because it's a new entity or because the preexisting_graph wasn't provided). Obviously, this is an unwanted behaviour.

@arcangelo7 : thank you again for spotting all these issues... I'm going to fix this and then a new version will be published!

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.

2 participants