Allow clearing default material libraries by assigning `None` #802

Vikranth3140 · 2025-09-03T22:48:24Z

Description

This PR implements support for making material and problem-level default libraries “None-able,” addressing Issue #801.

Summary of changes

Updated __setitem__ for default_libraries mapping:
- Assigning None to a default library key (e.g., "nlib", "plib", or LibraryType.NEUTRON) now unsets/deletes the entry.
- Non-None assignments remain backward-compatible (Library or str).
Updated update() so that passing {"nlib": None} or keyword nlib=None also unsets correctly.
Serializer logic verified to omit unset defaults (no "None" literal is emitted).
Adjusted type checks and docstrings to clarify accepted values (Library | str | None).

Behavior

Before:
- Reading a missing default returned None.
- But assigning None raised an error or resulted in an invalid literal "None" in output.
After:
- Reading a missing default still returns None.
- Assigning None now unsets the default, and it is omitted from serialization.

Fixes #801

General Checklist

I have performed a self-review of my own code.
The code follows the standards outlined in the [development documentation](https://www.montepy.org/en/stable/dev_standards.html).
I have formatted my code with black version 25.
I have added tests that prove my fix is effective or that my feature works (if applicable).

Documentation Checklist

I have documented all added classes and methods.
For infrastructure updates, I have updated the developer's guide.
For significant new features, I have added a section to the getting started guide.

First-Time Contributor Checklist

If this is your first contribution, add yourself to pyproject.toml if you wish to do so.

Additional Notes for Reviewers

Ensure that:

The submitted code is consistent with the merge checklist outlined [here](https://www.montepy.org/en/stable/dev_checklist.html#merge-checklist).
The PR covers all relevant aspects according to the development guidelines.
100% coverage of the patch is achieved, or justification for a variance is given.

… with None

…material and problem scopes

…r clearing a default library by assigning None.

MicahGale · 2025-09-04T20:22:39Z

Thanks @Vikranth3140 for this contribution. I'm a bit busy right now and it will be a little while before I can review this.

Did you use the help of an LLM (e.g., ChatGPT, claude, Copilot, etc.) to prepare this PR? We are still working on our exact policy, but using LLMs is fine as long as this use is disclosed.

MicahGale · 2025-09-04T20:25:37Z

This is not part of my review, I just noticed that on coveralls I noticed the patch coverage is 80%. We really aim for 100% patch coverage unless a good reason this isn't possible is provided. I thought while you wait on my review you could work on the test coverage.

Vikranth3140 · 2025-09-04T20:29:15Z

Thanks @Vikranth3140 for this contribution. I'm a bit busy right now and it will be a little while before I can review this.

Did you use the help of an LLM (e.g., ChatGPT, claude, Copilot, etc.) to prepare this PR? We are still working on our exact policy, but using LLMs is fine as long as this use is disclosed.

Hi, Yes, I had written the base code and optimized it using Copilot and also written the tests/test_material.py using it.

Vikranth3140 · 2025-09-04T20:32:16Z

This is not part of my review, I just noticed that on coveralls I noticed the patch coverage is 80%. We really aim for 100% patch coverage unless a good reason this isn't possible is provided. I thought while you wait on my review you could work on the test coverage.

Hi, so the add_rtd_link is failing because my workflow permissions are read-only and does not accept accept modifications from a PR

MicahGale · 2025-09-04T21:35:00Z

This is not part of my review, I just noticed that on coveralls I noticed the patch coverage is 80%. We really aim for 100% patch coverage unless a good reason this isn't possible is provided. I thought while you wait on my review you could work on the test coverage.

Hi, so the add_rtd_link is failing because my workflow permissions are read-only and does not accept accept modifications from a PR

Here is the documentation build: https://montepy--802.org.readthedocs.build/en/802/

Yes I know about this issue and I don't know of a good workaround yet. It's completely fine to ignore that failure though.

Vikranth3140 · 2025-09-07T13:44:17Z

This is not part of my review, I just noticed that on coveralls I noticed the patch coverage is 80%. We really aim for 100% patch coverage unless a good reason this isn't possible is provided. I thought while you wait on my review you could work on the test coverage.

Hi, so the add_rtd_link is failing because my workflow permissions are read-only and does not accept accept modifications from a PR

Here is the documentation build: https://montepy--802.org.readthedocs.build/en/802/

Yes I know about this issue and I don't know of a good workaround yet. It's completely fine to ignore that failure though.

Okay cool

Vikranth3140 · 2025-09-14T16:05:26Z

Hi @MicahGale, is there any update on this PR

MicahGale

This still needs some improvements.

This does not fix #801, but rather #800.
Some editorial changes for clarity
The new update method is not necessary for #800. It could be helpful, but currently is not being effectively tested. So I would say either remove it or make the testing more thorough to get 100% coverage.

Coverage report because coveralls is currently down. The current CI failure is due to Coveralls being done, so you don't need to worry about it.
htmlcov.zip

MicahGale · 2025-09-14T16:01:41Z

doc/source/changelog.rst


+**Features Added**
+
+* Allow clearing default libraries by assigning ``None`` at material and problem scopes; unset values are omitted from serialization.


I do not follow the "unset values are omitted from serialization" aspect, and I'm not sure it's true. Also I don't think such a technical detail is needed in a changelog.

This needs to be linked to an issue well. See the below bug fix.

MicahGale · 2025-09-14T16:08:56Z

montepy/data_inputs/material.py

+    def update(self, other=None, **kwargs):
+        """Update the default libraries with the key/value pairs from other, overwriting existing keys.
+
+        Parameters
+        ----------
+        other : dict or dict-like
+            Another dict or dict-like object to update from
+        **kwargs
+            Additional key-value pairs to update
+        """


This doc string is rather generic, and is not very specific to this class. It might benefit from an example. I could imagine it being used in the following way (which is not a full example):

mat.default_libraries.update({"nlib": "80c", "plib": "80p"})

That being said this is not required for fixing #800, and could be dropped from this PR if needed.

MicahGale · 2025-09-14T16:13:34Z

montepy/data_inputs/material.py

+        if other is not None:
+            if hasattr(other, "items"):


This is a user-facing function, so the inputs cannot be trusted. A lot of the type and value enforcement will be handled by __setitem__, however you will need to ensure that other is a dictionary prior to iterating over it.

MicahGale · 2025-09-14T16:16:18Z

montepy/data_inputs/material.py

    :func:`~montepy.data_inputs.material.Material.default_libraries` acts like a dictionary,
    and can accept a string or a :class:`~montepy.particle.LibraryType` as keys.

+    To clear a default library, assign ``None``:


Suggested change

To clear a default library, assign ``None``:

A default library can be cleared by assigning it to be ``None``:

MicahGale · 2025-09-14T16:18:07Z

montepy/data_inputs/material.py

        :func:`~montepy.data_inputs.material.Material.default_libraries` acts like a dictionary,
        and can accept a string or a :class:`~montepy.particle.LibraryType` as keys.

+        To clear a default library, assign ``None``:


Suggested change

To clear a default library, assign ``None``:

A default library can be cleared by assigning it to be ``None``:

MicahGale · 2025-09-14T16:21:02Z

montepy/materials.py

        To set the default libraries for a problem you need to set this dictionary
-        to a Library or string.
+        to a Library or string. To clear a default library, assign ``None``.


Suggested change

To set the default libraries for a problem you need to set this dictionary

to a Library or string.

to a Library or string. To clear a default library, assign ``None``.

To set the default libraries for a problem you need to set the values of this dictionary

to a Library or string.

to a Library or string. To clear a default library, set the value to ``None``.

MicahGale · 2025-09-14T16:39:31Z

tests/test_material.py

+    def test_default_libraries_none_setting(_, dl):
+        """Test that setting default libraries to None unsets them."""
+        # Set a library first
+        dl["plib"] = "80p"
+        assert dl["plib"] == Library("80p")


As discussed the patch coverage is currently 80% so more testing is needed. I suspect most of the needed testing is for more atypical ways to call these changes.

MicahGale · 2025-09-14T16:40:38Z

tests/test_material.py

+        # Test update method with None
+        dl["nlib"] = "00c"
+        dl["plib"] = "80p"
+        dl.update({"plib": None})


This only tests one way that this can be called. You need to also test:

calling it with just kwargs

Calling it with other and kwargs

calling with not a dictionary

other=None

etc.

Vikranth3140 added 5 commits September 4, 2025 04:00

Add update method to _DefaultLibraries and support clearing libraries…

455b9db

… with None

Add feature to allow clearing default libraries by assigning None at …

ab5ec7c

…material and problem scopes

Clarify documentation on setting default libraries and add example fo…

c30283b

…r clearing a default library by assigning None.

Add tests for unsetting default libraries by assigning None

81a3e8f

Add Vikranth Udandarao to authors list in pyproject.toml

ddb420f

Vikranth3140 force-pushed the develop branch from 2af789b to ddb420f Compare September 4, 2025 03:08

run black formatting

5eb8b3f

Merge branch 'develop' into develop

69772a7

MicahGale requested changes Sep 14, 2025

View reviewed changes


		Features Added

		* Allow clearing default libraries by assigning ``None`` at material and problem scopes; unset values are omitted from serialization.

	To clear a default library, assign ``None``:
	A default library can be cleared by assigning it to be ``None``:

Allow clearing default material libraries by assigning None #802

Are you sure you want to change the base?

Allow clearing default material libraries by assigning None #802

Uh oh!

Conversation

Vikranth3140 commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

General Checklist

Documentation Checklist

First-Time Contributor Checklist

Additional Notes for Reviewers

Uh oh!

MicahGale commented Sep 4, 2025

Uh oh!

MicahGale commented Sep 4, 2025

Uh oh!

Vikranth3140 commented Sep 4, 2025

Uh oh!

Vikranth3140 commented Sep 4, 2025

Uh oh!

MicahGale commented Sep 4, 2025

Uh oh!

Vikranth3140 commented Sep 7, 2025

Uh oh!

Vikranth3140 commented Sep 14, 2025

Uh oh!

MicahGale left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Allow clearing default material libraries by assigning `None` #802

Allow clearing default material libraries by assigning `None` #802

Vikranth3140 commented Sep 3, 2025 •

edited

Loading

MicahGale left a comment •

edited

Loading