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

Rename classes #751

Merged
merged 30 commits into from
Jun 30, 2023
Merged

Rename classes #751

merged 30 commits into from
Jun 30, 2023

Conversation

liamhuber
Copy link
Member

Rename Node to Function and then IsNodal to Node, including downstream changes like changing the decorator from @node to @function_node, the children FastNode to just Fast, etc.

In the process of updating the example notebook, I also made two small fixes to problems that had slipped through the cracks because they didn't raise any errors:

  • Changing the workflow=wf kwarg to parent=wf in one of the instantiations
  • Changing some of the update/run stuff in cell 17 to reflect what happens by default.

Not included in this PR: Moving all the docstrings around so that they better conform to reality and the spec in #743 -- right now Function(Node) has a bunch of pretty general specifications that should probably be moved over to Node, and the idea of macro nodes is not yet mentioned.

Including a couple of small fixes that are unrelated and needed to be done, but didn't result in errors so hadn't been caught by the CI: Changing the `workflow` kwarg to `parent` in a node instantiation, and updating the description of how things are (or aren't) automatically updated in cell 17.
@liamhuber liamhuber added the .workflow Pertaining to the workflow submodule label Jun 28, 2023
@liamhuber liamhuber requested a review from samwaseda June 28, 2023 21:57
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions
Copy link
Contributor

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_contrib/rename_classes

@liamhuber liamhuber changed the base branch from main to fix_type_hints June 28, 2023 21:58
# Conflicts:
#	pyiron_contrib/workflow/is_nodal.py
It's not exhaustive or finalized, but in general the universal stuff now sits on `Node` and the stuff specific to wrapping functions sits on `Function(Node)`
@samwaseda
Copy link
Member

Is there a possibility to run tests here? It looks fine but without tests it's difficult to say...

@liamhuber
Copy link
Member Author

liamhuber commented Jun 30, 2023

Is there a possibility to run tests here? It looks fine but without tests it's difficult to say...

We could rebase this onto main briefly, which will trigger tests on this (including everything this is stacked on top of), or we can change the centralized CI as you and I propose to do/discuss in the Monday meeting.

For each of this stack of PRs I made sure the workflow tests are passing on my local machine; it will still be necessary to do the full CI on other architectures, but I guess it's already quite promising. Not running it here just delays catching problems until we get down to the main-most of the stacked PRs, but we'll still catch them with our current workflow.

@liamhuber and others added 3 commits June 30, 2023 09:24
To avoid cluttering the tab completion menu
Data channel value starts as not data
@liamhuber
Copy link
Member Author

It looks fine but without tests it's difficult to say...

Since the tests are running on my machine and we're not merging into main here, I'm going to lean into the it looks fine part of that comment and boldly merge.

@liamhuber liamhuber merged commit 6d897e9 into fix_type_hints Jun 30, 2023
@delete-merged-branch delete-merged-branch bot deleted the rename_classes branch June 30, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.workflow Pertaining to the workflow submodule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants