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

Remove concordance model and related code #809

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

dchiller
Copy link
Collaborator

@dchiller dchiller commented Jan 9, 2024

CAO Concordance information has historically been imported from Cantus Database but never used in Cantus Ultimus. Now that CAO concordances are no longer present in CantusDB (see DDMAL/CantusDB#431), we need to remove them from Cantus Ultimus. As discussed in #797, if this concordance information is ever desired in Cantus Ultimus, it would need to be re-added from Cantus Index.

This PR:

  • removes the Concordance model and references to it in other models
  • removes portions of commands related to the importing of concordance information
  • updates tests and test data to account for these removals
  • removes references to concordances in the solr schema and search frontend

Also, historically, migrations have not been committed to version control (see #562). Migrations should be committed to version control, so we take the opportunity with this model change to do so, removing migrations from .gitignore

Closes #797. Closes #562.

@dchiller dchiller changed the title Remove concordance model and related code Remove concordances model and related code Jan 9, 2024
@dchiller dchiller changed the title Remove concordances model and related code Remove concordance model and related code Jan 9, 2024
@@ -5,9 +5,8 @@ class Chant(models.Model):
"""
A Chant belongs to a image page (or a chant can appear
on multiple pages?)

Choose a reason for hiding this comment

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

at some point, it might be nice to have a definitive statement here, rather than a question :P

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, i would also like to know how we will solve this problem 😂😂

Copy link

@jacobdgm jacobdgm Jan 11, 2024

Choose a reason for hiding this comment

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

Like, in practice, I know a chant can span multiple pages.

Here, could we say something along the lines of, "a Chant belongs to an image page. In physical manuscripts, the last chant on a page often continues onto the next page. Cantus Database associates each chant with the page on which it begins, and Cantus Ultimus follows this convention [or substitute whatever CU actually does]"? A statement like this would communicate some of the nuance of the situation and how CU currently handles it, as compared to the current statement which communicates something along the lines of "it's complicated and weird, and darned if I know what to do about it!".

Copy link

@jacobdgm jacobdgm left a comment

Choose a reason for hiding this comment

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

Everything here looks fine to me. I left a couple of comments, but nothing that needs to be addressed before merging.

@dchiller dchiller merged commit adec12c into DDMAL:main Jan 11, 2024
2 checks passed
@dchiller dchiller deleted the i-797-remove-cao-concordances branch January 11, 2024 14:47
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.

Remove dependence on CAO concordances Explore including django migrations in version control
2 participants