-
Notifications
You must be signed in to change notification settings - Fork 1
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
updated ismi sample data for development branch #104
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
📋 Review Summary
- Number of files reviewed: 2
- Number of comments: 0
- Number of suggestions: 0
📚 File changes
File path | File changes |
---|---|
examples/use-cases/ismi/README.md | Updated the ISMI sample data structure and file references. |
examples/use-cases/ismi/data/ismi-crm-date-samples.ttl | Updated ISMI sample data for the development branch. |
Ignored comments
examples/use-cases/ismi/README.md
-
refactor_suggestion: The previous description of the old ISMI database and its structure has been removed. Ensure that the new structure is clearly explained and that any necessary context about the transition from the old to the new database is provided elsewhere in the documentation if needed.
-
refactor_suggestion: The new structure of the ISMI database is introduced without a transition explanation. Consider adding a brief note about the significance of this change and how it improves upon the previous structure for clarity.
-
refactor_suggestion: The sample file reference has changed from a JSON format to a TTL format. Ensure that users are aware of this change and provide guidance on how to work with the new file format if necessary.
examples/use-cases/ismi/data/ismi-crm-date-samples.ttl
- refactor_suggestion: Consider using a consistent format for the date labels across different calendar types to improve clarity and maintainability. For example, the Islamic dates use a specific format while Gregorian and Julian dates use a different one. Standardizing these formats can help in future data processing and understanding.
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
The updated example data and description looks great, and I think that part could be merged.
If you want to back out the notebook and move that to a different branch we could merge this portion - up to you.
I took out the notebook and moved it to an extra branch "ismi-sample-notebook". Do you want to review @rlskoeser or should I just merge? |
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.
Great! Thanks for updating & documenting.
We plan to extract as much as possible of this data in the migration to the new RDF database with a CIDOC-CRM-based data model. | ||
|
||
A sample file with dates of each type can be found in `data/ismi-om4-date-samples.json` | ||
A sample file with dates of each type can be found in `data/ismi-crm-date-samples.ttl` |
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.
We could turn this into a markdown relative link at some point, but this is fine for now. I think we're going to need to restructure the examples and notebooks anyway now that we have a few more of them.
update ISMI sample data for development branch.