Skip to content

Conversation

@ghukill
Copy link
Contributor

@ghukill ghukill commented Jun 3, 2025

Purpose and background context

This PR updates the TDA library, and all dependencies while at it.

This is achieved by updating the make update command to explicitly update the TDA library. This is not achieved by pipenv update --dev despite it being a dependency in the Pipfile.

There may be alternate approaches for the installation, versioning, and updating of local libraries from Git...but this feels like a pretty workable solution for the time being: one line in the Makefile ensures that TDA is updated alongside all other libraries.

FWIW, I would propose that our (hopeful) switch to uv would be a good time to revisit this approach, if we'd like to.

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

Manual testing is not terribly convenient for the pipeline lambda, but there are no plans for it to use the new run_timestamp column for any reads.

Includes new or updated dependencies?

YES: bumps timex-dataset-api to v.2.0 (current version)

Changes expectations for external applications?

YES: any read/writes to the dataset will include the run_timestamp column now

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:

The TDA library is a Github dependency in the Pipefile, but the
normal `pipenv update --dev` does not pickup versions bumps in the
TDA library.  However, an explicit `pipenv update <TDA_URL>` will.

How this addresses that need:
* Adds an explicit TDA update in the `make update` command

Side effects of this change:
* TDA is updated to v2.0 (current)

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/TIMX-496
@ghukill ghukill force-pushed the TIMX-496-update-tda branch from 16199f5 to be9ce95 Compare June 3, 2025 17:17
@ghukill ghukill requested a review from a team June 3, 2025 17:58
@ehanson8 ehanson8 self-assigned this Jun 3, 2025
@ghukill ghukill merged commit ff2b58a into main Jun 3, 2025
3 checks passed
@coveralls
Copy link

coveralls commented Jun 4, 2025

Pull Request Test Coverage Report for Build 15423664672

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 95.753%

Totals Coverage Status
Change from base Build 14892419559: 0.0%
Covered Lines: 248
Relevant Lines: 259

💛 - Coveralls

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