-
-
Notifications
You must be signed in to change notification settings - Fork 203
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: changed base location for graph_from_graphdb and added tests #1732
base: main
Are you sure you want to change the base?
Conversation
Current Aviator status
This pull request is currently open (not queued). How to mergeTo merge this PR, comment
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea!
I see a typo in the file "algothitms", it could be fixed at the same time.
I wonder whether the database should be cached in an user dir. Should the argument of where to save it be exposed?
I suggest hosting it under the igraph organization for the purposes of allowing the function to work. In our own copy, we should mention the license and list the original authors, in addition to linking to Zenodo. I'm really glads to see that this is on Zenodo given that the MIVIA website is gone now.
This dataset is way too large for the igraphdata package. I suggest using a separate repo, exactly what @schochastics did, but under the igraph organization. I believe you should be able to create a new repo, but if not let me know and I'll see if I can fix that. Can this function operate with a local copy of the database? Would this be easy to implement, and if yes, can we include an explanation of how to do it? Other than the comments above, this LGTM. |
I will try to host it under the igraph org and put all the necessary information into the README. The files have a CC license so that should all be fine. I also checked that it works with a local copy. You simply need to provide the path to the database. I will also update the docs so that this is clear |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these actually work and look better? Please merge when good and green.
79867f2
to
88ed1c9
Compare
Co-authored-by: Maëlle Salmon <[email protected]>
Co-authored-by: Kirill Müller <[email protected]>
ea68d02
to
261e9e3
Compare
Fix #1712
Downloaded the database from here: https://zenodo.org/records/11204020
and hosted it here: https://github.com/schochastics/graphsdb/
General question: should we host it somewhere else?