Skip to content

Clean up Regnum phylorefs#89

Merged
gaurav merged 16 commits intomasterfrom
fix-specifier-authorities
Nov 11, 2025
Merged

Clean up Regnum phylorefs#89
gaurav merged 16 commits intomasterfrom
fix-specifier-authorities

Conversation

@gaurav
Copy link
Copy Markdown
Member

@gaurav gaurav commented Jul 2, 2021

This PR cleans up Regnum phylorefs in several ways:

  • Renamed files from REGNUM_* to CLADO_* and increased the number of digits to seven.
  • Moved Phylonym Phyx files and scripts out of the encrypted/ directory so that they will no longer be encrypted (closes Unencrypt Phylonym files #82).
  • Created subdirectories in phyx/phylonym for two kinds of errors we've been running into: newick-problems and newick-recursion-error.
  • Added phyx2owl.js, a script to convert Phyx files to OWL for testing -- I'll replace this with the phyx.js library later.
  • Added phyxfix.js, a script to correct errors in curated Phyx files on the fly, and used it to fix specifier authorities that were not loaded correctly (i.e. before PR Improvements to regnum2phyx.js #96).
  • Incorporated some of the phylogenies prepared by Anna Becker.
  • Removed two files with failing Newick strings and two test clade definitions from Regnum.

@gaurav gaurav force-pushed the fix-specifier-authorities branch from 99b5364 to 8e18aff Compare July 2, 2021 04:56
@gaurav gaurav marked this pull request as ready for review July 13, 2021 00:09
@gaurav gaurav requested a review from hlapp July 13, 2021 00:09
@hlapp
Copy link
Copy Markdown
Member

hlapp commented Jul 13, 2021

Well, it sounds like there are two types of changes wrapped into one. By two types I mean a set of changes that does not lend itself to code review, and those that do. The overwhelming bulk (by "lines changed") seems to fall into the former category, suggesting that there is a small enough set in the latter category to still enable review.

As it is, this PR is unreviewable, so if we keep it we shouldn't pretend it was ever reviewed. It sounds to me though that the changes that don't stand to benefit from review (decrypting files, for example) can be separated out, even if they would necessarily have to be accompanied by some code changes (in the tests) too. But there seem to be additional code changes (items 2, 3, and 6 in your list) that are separable changes of code that seem reviewable and would benefit from having been reviewed.

@gaurav gaurav force-pushed the fix-specifier-authorities branch from d0d9886 to 07fd379 Compare November 10, 2025 04:45
@gaurav gaurav changed the base branch from master to reviewable-changes November 10, 2025 04:45
@gaurav gaurav force-pushed the fix-specifier-authorities branch from 07fd379 to 07da7e8 Compare November 10, 2025 04:59
@gaurav gaurav requested review from hlapp and removed request for hlapp November 10, 2025 05:08
@gaurav
Copy link
Copy Markdown
Member Author

gaurav commented Nov 10, 2025

@hlapp I've moved the reviewable changes into PR #96, so this PR should consist only of changes I made to encrypted files, and then the final change to unencrypt all the encrypted files in this repo.

Copy link
Copy Markdown
Member

@hlapp hlapp left a comment

Choose a reason for hiding this comment

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

As a note, same (I think) Unexpected console statement warnings from the linting step as in #96.

gaurav added a commit that referenced this pull request Nov 11, 2025
These are the reviewable changes from PR #89, which are primarily `regnum2phyx.js` changes:
- Improvements to how author names are rendered: we now support first-name first, last-name first, or last-name only.
- Generate scientific name authorship by using first-name first and combining them with " and ".
- Generate specifier authors by using last-name only and combining them with " and ".
- Added a CLI option to configure the number of digits in the `CLADO_` name, which now defaults to 7 instead of the previous 6.
- Modified the script to produce `CLADO_0000000` filenames instead of `REGNUM_0000000` filenames.

Two other minor changes:
- Added an optional test to ensure that Phyx files have at least one Newick phylogeny in order to see how many we have.
- Removed `phyloref_testcase.owl`, which was previously a copy of the phyloref.owl ontology we used to do reasoning.
Base automatically changed from reviewable-changes to master November 11, 2025 22:12
@gaurav gaurav merged commit 4de3244 into master Nov 11, 2025
6 checks passed
@hlapp hlapp deleted the fix-specifier-authorities branch November 12, 2025 00:16
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.

Unencrypt Phylonym files

2 participants