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 loading in windows, url paths #446

Merged
merged 1 commit into from
Aug 30, 2022
Merged

Conversation

francescalb
Copy link
Collaborator

@francescalb francescalb commented Aug 19, 2022

This works for me now in both windows and linux.
from ontopy import get_ontology
onto = get_ontology('https://raw.githubusercontent.com/BIG-MAP/BattINFO/master/battinfo.ttl').load()**

Can you please check?

Description:

Type of change:

  • Bug fix.
  • New feature.
  • Documentation update.

Checklist:

This checklist can be used as a help for the reviewer.

  • Is the code easy to read and understand?
  • Are comments for humans to read, not computers to disregard?
  • Does a new feature has an accompanying new test (in the CI or unit testing schemes)?
  • Has the documentation been updated as necessary?
  • Does this close the issue?
  • Is the change limited to the issue?
  • Are errors handled for all outcomes?
  • Does the new feature provide new restrictions on dependencies, and if so is this documented?

Comments:

@francescalb francescalb changed the title Test eq WIP: Fix loading in windows, url paths Aug 19, 2022
@francescalb francescalb changed the title WIP: Fix loading in windows, url paths Fix loading in windows, url paths Aug 19, 2022
@@ -350,7 +350,7 @@ def load_uri(uri, dirname):
url = f"{baseuri}/{uri_as_str}"
else:
url = os.path.join(baseuri if baseuri else dirname, uri_as_str)
url = normalise_url(url)
# url = normalise_url(url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this create other issues, since the URL is now not normalized?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both Jesper and I tried to see if we found such cases, but did not. The reason why it was commented out and not deleted completely was for easy reversal (and then searching for a new solution for Windows) if needed.

@sygout
Copy link
Collaborator

sygout commented Aug 22, 2022

I tried to run pytest on Windows and got: 5 failed, 17 passed, 2 skipped, 7 warnings

@francescalb
Copy link
Collaborator Author

I tried to run pytest on Windows and got: 5 failed, 17 passed, 2 skipped, 7 warnings

Yes, I do to on windows. We need to set up a testing environment for windows. Since I (almost) never use windows for this I have not encoutnered them. I suggest making a separate issue for creating a windows testing environment though. And we just make sure that loading works in both windows and linux for now

@sygout
Copy link
Collaborator

sygout commented Aug 22, 2022

On a fresh installation (new virtual environment and clean repository clone), the correction seems to work as test_load is working, the error present in master v0.3.1-19-g02b3c84 (FAILED test_load.py::test_load - urllib.error.HTTPError: HTTP Error 400: https://raw.githubusercontent.com/%5CBIG-MAP) does not appear anymore.

@francescalb francescalb merged commit 4e0e46d into master Aug 30, 2022
@francescalb francescalb deleted the issue316-Windowspaths branch August 30, 2022 07:39
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.

3 participants