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

Add final set of Datacite field methods #189

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

ehanson8
Copy link
Contributor

Purpose and background context

Finish refactoring Datacite transform to use field methods.

How can a reviewer manually see the effects of these changes?

Run the following command to see that the Datacite transform still transforms a source file:

pipenv run transform -i tests/fixtures/datacite/datacite_records.xml -o output/datacite-transformed-records.json -s jpal

Includes new or updated dependencies?

NO

Changes expectations for external applications?

NO

What are the relevant tickets?

Developer

  • All new ENV is documented in README
  • All new ENV has been added to staging and production environments
  • All related Jira tickets are linked in commit message(s)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer(s)

  • The commit message is clear and follows our guidelines (not just this PR message)
  • There are appropriate tests covering any new functionality
  • The provided documentation is sufficient for understanding any new functionality introduced
  • Any manual tests have been performed and verified
  • New dependencies are appropriate or there were no changes

Why these changes are being introduced:
* Finish refactoring Datacite transform to use field methods

How this addresses that need:
* Add field methods and associated private methods for notes, publishers, related_items, rights, subjects, and summary
* Add unit tests for new field methods

Side effects of this change:
* None

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/TIMX-284
Copy link
Contributor

@jonavellecuerdo jonavellecuerdo left a comment

Choose a reason for hiding this comment

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

Awesome work! Just a few questions/suggestions for ya'.

transmogrifier/sources/xml/datacite.py Outdated Show resolved Hide resolved
transmogrifier/sources/xml/datacite.py Outdated Show resolved Hide resolved
transmogrifier/sources/xml/datacite.py Show resolved Hide resolved
transmogrifier/sources/xml/datacite.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ghukill ghukill left a comment

Choose a reason for hiding this comment

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

Left a couple of suggestions, but nothing blocking from me. Looking good. And (pre)congrats on the first source in the books!

As mentioned, I think it's pretty exciting to see get_optional_fields() as a list of method calls, knowing this could very easily be refactored again (if decided we go that route) to a higher up calling context.

transmogrifier/sources/xml/datacite.py Show resolved Hide resolved
transmogrifier/sources/xml/datacite.py Outdated Show resolved Hide resolved
transmogrifier/sources/xml/datacite.py Outdated Show resolved Hide resolved
transmogrifier/sources/xml/datacite.py Outdated Show resolved Hide resolved
* Remove unnecessary list pattern from get_content_types, get_languages, and get_publishers methods
* Refactor _get_description_notes method to remove unnecessary var and add description_type var
* Refactor get_subjects method to use defaultdict
@ehanson8
Copy link
Contributor Author

ehanson8 commented Jun 4, 2024

@jonavellecuerdo Pushed a new commit!

@ehanson8 ehanson8 merged commit 0529253 into main Jun 5, 2024
5 checks passed
@ehanson8 ehanson8 deleted the TIMX-284-datacite-fmr-beyond-thunderdome branch June 5, 2024 14:04
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