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

Accessing Project from Job #423

Closed
javierbg opened this issue Nov 24, 2020 · 13 comments
Closed

Accessing Project from Job #423

javierbg opened this issue Nov 24, 2020 · 13 comments
Labels
enhancement New feature or request good first issue Good for newcomers pinned Instructs stale bot to ignore this issue.
Milestone

Comments

@javierbg
Copy link
Contributor

Feature description

Sometimes I need to access the Project object from a job, for example to access the project-wide data attribute. Currently, in order to achieve this, a module-wide variable has to be created for the project, but this could span out over several modules and need to load/import the project from each one.

Proposed solution

Exposing the already present job._project as job.project. If this is not possible in the near future, the current way of doing things could be explained in the docs (I was not able to find it, if it is not there I could add it myself).

@javierbg javierbg changed the title Accessing Project from job Accessing Project from Job Nov 24, 2020
@csadorf csadorf added this to the v2.0.0 milestone Nov 24, 2020
@csadorf csadorf added the enhancement New feature or request label Nov 24, 2020
@csadorf
Copy link
Contributor

csadorf commented Nov 24, 2020

@javierbg Thank you for reporting! 👍

@bdice
Copy link
Member

bdice commented Nov 30, 2020

@javierbg I agree this feature would be useful and we may implement it as you described in signac 2.0. There are some edge cases that could be problematic if we changed this currently, but for most cases job._project will provide the project that you want. The project developers have had several discussions in the past about the project/job relationship, not all of which are documented on GitHub. We would likely make this change in signac 2.0 depending on the outcomes of some relevant issues like #390, #96. Feel free to join our Slack community if you want to learn more, contribute to the package, or discuss this issue during a developer meeting! 😃 https://signac.io/slack-invite/

@tcmoore3
Copy link
Member

See also #96

@stale
Copy link

stale bot commented Apr 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 18, 2021
@vyasr vyasr removed the stale label Apr 18, 2021
@stale
Copy link

stale bot commented Jun 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 18, 2021
@csadorf csadorf added the pinned Instructs stale bot to ignore this issue. label Jun 18, 2021
@stale stale bot removed the stale label Jun 18, 2021
@vyasr
Copy link
Contributor

vyasr commented Mar 14, 2022

@glotzerlab/signac-committers should we revisit this going into signac 2.0? It would be possible to change this now, does anyone see any downsides?

@bdice
Copy link
Member

bdice commented Mar 14, 2022

I have spent some time thinking about this and can’t think of any downsides.

I propose the project property is purposefully public, prior to publishing post-1.x packages. (Alliteration is fun.)

@b-butler
Copy link
Member

I personally use job._project in my projects for things like project default values.

@cbkerr
Copy link
Member

cbkerr commented Mar 18, 2022

I can see another use case: saving a plot from an aggregate back to the project directory for display using the dashboard project overview.

@bdice
Copy link
Member

bdice commented Mar 18, 2022

Awesome. Here is a proposal for implementation:

  1. On the next branch, edit signac/contrib/job.py to include the following:
@property
def project(self):
    """Return the project that contains this job."""
    return self._project
  1. Add tests to ensure that the project is what we expect, and that the project attribute cannot be assigned (should raise).
  2. Add the project property to the documentation.

As an extension of this proposal, we could make assignment of the project property trigger a “move” of the job. We could do that by adding a setter, and we already have methods to move jobs from one project to another that handle the project cache invalidation correctly. We would also have to invalidate the job’s cached properties.

I’ll label this a good first issue since I don’t anticipate any major design challenges in the first portion of the implementation proposal for the property.

@bdice bdice added the good first issue Good for newcomers label Mar 18, 2022
@vyasr
Copy link
Contributor

vyasr commented Apr 12, 2022

The only change I would make to the above instructions is that we'll want to branch off of schema2, not next (or wait for schema2 to be merged into next). As long as the workspace directory is configurable the job->project mapping isn't quite valid.

While I am still in favor of this, and all the necessary upstream changes have now been made to enable this feature, I do not think this is a blocker for 2.0. This is a nice-to-have feature, but it's not critical and there's no reason that it couldn't be added in a minor release. Especially since we've marked it as a good first issue I would like to leave it available rather than pushing to complete it for 2.0

@bdice
Copy link
Member

bdice commented Apr 20, 2022

Following up: schema2 has been merged into next, so this work should begin from the next branch.

This was referenced Aug 10, 2022
@vyasr
Copy link
Contributor

vyasr commented Aug 30, 2022

Resolved by #808.

@vyasr vyasr closed this as completed Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers pinned Instructs stale bot to ignore this issue.
Projects
None yet
Development

No branches or pull requests

7 participants