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

Refactor a lot of paths to re-organize structure #295

Merged
merged 12 commits into from
Nov 4, 2020
Merged

Conversation

JackUrb
Copy link
Contributor

@JackUrb JackUrb commented Oct 29, 2020

As noted in #285, updating the paths. The goal here is to better follow the overall architecture design document. Most extra lines come from newly committed lock files. We're moving to this format so that the system as a whole is more understandable as we look to finalize components of the design. Nothing has been deleted in this PR, just moved, so all existing code should still work. If you're running on this tag though and see warnings, that means you'll need to make some changes before updating to v0.3.

How to switch over?

If you're reading this, it's likely because you're about to do a bunch of find replaces in order to ensure that your code is now up to the new structure. Following this list of find-replaces will likely help. Here's a list of the changes, in order of likely relevance:

  • mephisto.core.local_database is now mephisto.abstractions.databases.local_database
  • mephisto.core.data_browser is now mephisto.tools.data_browser
  • the rest of mephisto.core. is now at mephisto.operations.
  • mephisto.utils.scripts is now mephisto.tools.scripts
  • mephisto.data_model.database is now mephisto.abstractions.database
  • mephisto.server.blueprints. is now mephisto.abstractions.blueprints.
  • mephisto.server.architects. is now mephisto.abstractions.architects.
  • mephisto.providers. is now mephisto.abstractions.providers.
  • mephisto.data_model.assignment.Unit has been broken out into mephisto.data_model.unit
  • mephisto.data_model.task.TaskRun has been broken out into mephisto.data_model.task_run
  • mephisto.data_model.assignment_state is now mephisto.data_model.constants.assignment_state
  • mephisto.server.channels.channel is now mephisto.abstractions.channel
  • mephisto.server.channels.websocket_channel is now mephisto.abstractions.architects.channels.websocket_channel
  • mephisto.data_model.architect is now mephisto.abstractions.architect
  • mephisto.data_model.crowd_provider is now mephisto.abstractions.crowd_provider
  • mephisto.data_model.blueprint is now mephisto.abstractions.blueprint

TODO

  • move the files
  • run the tests before updating callsites
  • run the tests after updating callsites
  • document how to transition
  • rebase on master

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 29, 2020
@JackUrb JackUrb requested a review from pringshia October 30, 2020 20:30
@JackUrb JackUrb changed the title [WIP] Refactor a lot of paths to re-organize structure Refactor a lot of paths to re-organize structure Oct 30, 2020
@JackUrb JackUrb marked this pull request as ready for review October 30, 2020 20:32
@codecov-io
Copy link

codecov-io commented Oct 30, 2020

Codecov Report

Merging #295 into master will increase coverage by 0.52%.
The diff coverage is 62.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #295      +/-   ##
==========================================
+ Coverage   66.48%   67.00%   +0.52%     
==========================================
  Files          74       74              
  Lines        6611     6526      -85     
==========================================
- Hits         4395     4373      -22     
+ Misses       2216     2153      -63     
Impacted Files Coverage Δ
mephisto/data_model/constants/__init__.py 100.00% <ø> (ø)
mephisto/data_model/project.py 57.69% <0.00%> (ø)
...ephisto/abstractions/providers/mturk/mturk_unit.py 18.51% <18.51%> (ø)
...blueprints/parlai_chat/parlai_chat_task_builder.py 18.86% <18.86%> (ø)
...phisto/abstractions/providers/mturk/mturk_utils.py 20.14% <20.14%> (ø)
.../blueprints/parlai_chat/parlai_chat_agent_state.py 25.75% <25.75%> (ø)
...histo/abstractions/providers/mturk/mturk_worker.py 28.57% <28.57%> (ø)
mephisto/data_model/agent.py 68.14% <28.57%> (-0.26%) ⬇️
...eprints/abstract/static_task/static_agent_state.py 30.00% <30.00%> (ø)
.../blueprints/parlai_chat/parlai_chat_task_runner.py 30.70% <30.70%> (ø)
... and 119 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5c262f...0837c8d. Read the comment docs.

@pringshia
Copy link
Contributor

Looks good to me, glad we have tests for situations like this!
Unfortunately we will lose our git history / git blameability with a lot of files (unsure if git mv would've preserved).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants