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

Add functional style methods to HasStructure #854

Merged
merged 10 commits into from
Jan 6, 2023
Merged

Add functional style methods to HasStructure #854

merged 10 commits into from
Jan 6, 2023

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Nov 5, 2022

This adds some transform/filter methods to HasStructure to allow a "functional programming" style in its use. Contingent on #853 and some tests/docs.

@stale
Copy link

stale bot commented Nov 23, 2022

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 Nov 23, 2022
@stale stale bot removed the stale label Nov 23, 2022
@coveralls
Copy link

coveralls commented Nov 23, 2022

Pull Request Test Coverage Report for Build 3622667903

  • 17 of 28 (60.71%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.008%) to 68.654%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pyiron_atomistics/atomistics/job/atomistic.py 0 1 0.0%
pyiron_atomistics/atomistics/structure/has_structure.py 15 25 60.0%
Totals Coverage Status
Change from base Build 3622625190: -0.008%
Covered Lines: 12116
Relevant Lines: 17648

💛 - Coveralls

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@pmrv pmrv added the integration Start the notebook integration tests for this PR label Nov 23, 2022
Apparently that throws an unrelated error
@pmrv pmrv marked this pull request as ready for review November 23, 2022 11:33
Copy link
Member

@liamhuber liamhuber left a comment

Choose a reason for hiding this comment

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

lgtm

pyiron_atomistics/atomistics/structure/has_structure.py Outdated Show resolved Hide resolved
@liamhuber
Copy link
Member

The rest of the CI needs to be triggered, but now that this isn't a draft, I bet the black formatting commit will trigger it.

@pmrv pmrv added the format_black reformat the code using the black standard label Nov 25, 2022
@stale
Copy link

stale bot commented Dec 21, 2022

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 Dec 21, 2022
@stale stale bot closed this Jan 5, 2023
@pmrv pmrv reopened this Jan 5, 2023
Performance impact is beyond negligible
@pmrv pmrv merged commit b0190c4 into main Jan 6, 2023
@delete-merged-branch delete-merged-branch bot deleted the structure_collect branch January 6, 2023 14:55
@jan-janssen jan-janssen restored the structure_collect branch January 7, 2023 06:13
@jan-janssen
Copy link
Member

It seems like this pull request broke the documentation jobs.

@pmrv
Copy link
Contributor Author

pmrv commented Jan 7, 2023

Since both #921 and #920 also fail docs, I'm not sure if this one is the cause. I'll see if I can reproduce the issue locally.

@jan-janssen
Copy link
Member

Since both #921 and #920 also fail docs, I'm not sure if this one is the cause. I'll see if I can reproduce the issue locally.

Yes, I agree ... still it is a bit frustrating that it is unrelated as it makes debugging even more complicated.

@pmrv
Copy link
Contributor Author

pmrv commented Jan 7, 2023

Here's the full error log from sphinx

# Platform:         linux; (Linux-6.1.1-arch1-1-x86_64-with-glibc2.36)
# Sphinx version:   6.1.1
# Python version:   3.11.0 (CPython)
# Docutils version: 0.19
# Jinja2 version:   3.1.2
# Pygments version: 2.14.0

# Last messages:
#   reading sources... [ 91%] apidoc/pyiron_atomistics.yaff.yaff
#   reading sources... [ 91%] index
#   reading sources... [ 92%] source/about
#   reading sources... [ 92%] source/examples
#   reading sources... [ 92%] source/faq
#   reading sources... [ 93%] source/import
#   reading sources... [ 93%] source/indices
#   reading sources... [ 93%] source/installation
#   reading sources... [ 94%] source/notebooks/QHA
#   reading sources... [ 94%] source/notebooks/Thermodynamics_QHA

# Loaded extensions:
#   sphinx.ext.mathjax (6.1.1)
#   sphinxcontrib.applehelp (1.0.2)
#   sphinxcontrib.devhelp (1.0.2)
#   sphinxcontrib.htmlhelp (2.0.0)
#   sphinxcontrib.serializinghtml (1.1.5)
#   sphinxcontrib.qthelp (1.0.3)
#   alabaster (0.7.12)
#   nbsphinx (0.8.11)
#   sphinx.ext.autodoc.preserve_defaults (6.1.1)
#   sphinx.ext.autodoc.type_comment (6.1.1)
#   sphinx.ext.autodoc.typehints (6.1.1)
#   sphinx.ext.autodoc (6.1.1)
#   sphinx.ext.viewcode (6.1.1)
#   sphinx.ext.autosummary (6.1.1)
#   sphinx.ext.napoleon (6.1.1)

# Traceback:
Traceback (most recent call last):
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/cmd/build.py", line 284, in build_main
    app.build(args.force_all, args.filenames)
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/application.py", line 347, in build
    self.builder.build_update()
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 311, in build_update
    self.build(to_build,
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 327, in build
    updated_docnames = set(self.read())
                           ^^^^^^^^^^^
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 434, in read
    self._read_serial(docnames)
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 455, in _read_serial
    self.read_doc(docname)
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/builders/__init__.py", line 511, in read_doc
    publisher.publish()
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/docutils/core.py", line 226, in publish
    self.apply_transforms()
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/docutils/core.py", line 206, in apply_transforms
    self.document.transformer.apply_transforms()
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/transforms/__init__.py", line 80, in apply_transforms
    super().apply_transforms()
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/docutils/transforms/__init__.py", line 173, in apply_transforms
    transform.apply(**kwargs)
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/transforms/__init__.py", line 206, in apply
    apply_source_workaround(node)
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/util/nodes.py", line 175, in apply_source_workaround
    node.source = get_node_source(node) or ''
                  ^^^^^^^^^^^^^^^^^^^^^
  File "/home/ponder/micromamba/envs/pyiron_atomistics_docs/lib/python3.11/site-packages/sphinx/util/nodes.py", line 273, in get_node_source
    raise ValueError("node source not found")
ValueError: node source not found

It sounds like that's actually an issue with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black reformat the code using the black standard integration Start the notebook integration tests for this PR stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants