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

[IO-1754] Darwin-py v2 Dogfooding refactors #670

Merged
merged 17 commits into from
Oct 1, 2023
Merged

[IO-1754] Darwin-py v2 Dogfooding refactors #670

merged 17 commits into from
Oct 1, 2023

Conversation

Nathanjp91
Copy link
Contributor

@Nathanjp91 Nathanjp91 commented Sep 27, 2023

Problem

Dogfooding of darwin-py v2 code revealed some streamlining and refactors to make

Solution

List of things changed

  • Meta objects become normally named
  • Core objectes changed to use Core on the name
  • MetaClient becomes regular client and consistency with naming enforced
  • Optionality on _item with query and Core objects removed
  • _item renamed to _entity to avoid confusion with future 'item' development
  • Asserts regarding optionality removed
  • Cleanup of Mypy errors that had accumulated with changes over time
  • Modules with sub-sections now export * in the init.py file
  • Backend code structure consistently applied to Team backend
  • Collect function of query now has ability to force refresh on cache and will invalidate the cache on changes to the query
  • Exceptions streamlined by moving to singular module
  • Added Some exceptions in code more specific to darwin

Changelog

Darwin py v2 futures module refactored for streamlined useage

@linear
Copy link

linear bot commented Sep 27, 2023

IO-1754 Renaming Refactor

  • Rename _items [IO-1712]
  • Rename MetaObjects to Object and current core objects to CoreObjects [IO-1727]
  • Rename MetaClient to Client [IO-1462]

@Nathanjp91 Nathanjp91 marked this pull request as ready for review September 28, 2023 13:26
Copy link
Contributor

@owencjones owencjones left a comment

Choose a reason for hiding this comment

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

Largely straightforward, one comment to consider, but likely not act on

@@ -0,0 +1,4 @@
from darwin.future.core.datasets.create_dataset import *
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, I tend to do these specifically, e.g.

from module import Thing1, Thing2, Thing3

as it can avoid dep issues later, with name clashes and such.

I'd leave it for now unless you have good chance to fix it.

@@ -0,0 +1,5 @@
# Can't import * in this module because of a circular import problem specific to teams
Copy link
Contributor

Choose a reason for hiding this comment

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

Specific import might fix this, but again, don't break your back on it.



def get_team_raw(session: Session, url: str) -> JSONType:
"""Returns the team with the given slug in raw JSON format"""
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll ticket including returns and params in doc blocks, but leave for now

@Nathanjp91 Nathanjp91 merged commit 90ec58b into master Oct 1, 2023
9 checks passed
owencjones pushed a commit that referenced this pull request Oct 10, 2023
* client refactor

* item object rename

* delete/create classmethods

* core item rename

* meta object rename

* Client naming consistency

* remove useless asserts

* remove useless asserts

* collect refactor

* mypy cleanup

* streamlining

* expanding on exceptions

* __init__.py backend import changes

* team backend restructure for circular reference

* comment

* changes to collect operation, now invalidates cache if adding to query

* restructure exceptions module
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.

None yet

2 participants