Skip to content

Testing catalog build locally doesn't mistakenly retain old games#922

Open
edwardchalstrey1 wants to merge 9 commits into
masterfrom
fix/920
Open

Testing catalog build locally doesn't mistakenly retain old games#922
edwardchalstrey1 wants to merge 9 commits into
masterfrom
fix/920

Conversation

@edwardchalstrey1
Copy link
Copy Markdown
Member

@edwardchalstrey1 edwardchalstrey1 commented Jun 4, 2026

Issues closed by this PR

Description of the changes in this PR

This PR fixes stale catalog_data/ files persisting after pip install -e . by swapping the lookup priority in src/pygambit/catalog.py: the repo's live (and crucially differently named) catalog/ directory is now preferred over the installed catalog_data/, with catalog_data/ as the fallback for deployed (non editable install) packages, which will lack the catalog/ dir.

This ensures catalog changes are always picked up immediately in a editable install development checkout without reinstalling, and eliminates divergence between local and CI environments caused by leftover files from previous installs.

Note: non-editable installs with pip install . should already replace the catalog data correctly due to this logic in setup.py:


class GambitBuildPy(setuptools.command.build_py.build_py):
    """Extend `build_py` to copy the catalog games data into the build library."""

    def run(self) -> None:
        super().run()

        catalog_source = pathlib.Path("catalog")
        catalog_target = pathlib.Path(self.build_lib) / "pygambit/catalog_data"
        if catalog_target.exists():
            # Wipe the old catalog_data/ entirely
            shutil.rmtree(catalog_target)
        catalog_target.mkdir(exist_ok=True, parents=True)
        # Fresh copy of catalog_data/ from catalog/
        shutil.copytree(catalog_source, catalog_target, dirs_exist_ok=True)

There is also a new clean_stale_img_files function added which gets rid of anything in catalog/img which doesn't match the structure of catalog

How to review this PR

  • Test that when making changes to the catalog in an editable install and then doing pygambit.catalog.games() the changes are picked up
  • When running the update script, any files and folders in catalog/img from old games that were removed are removed

@rahulsavani
Copy link
Copy Markdown
Member

Unfortunately, this does not work yet.

My best guess is that the repo's live catalog/img directory contains everything from previous versions and is not getting cleared out. If that is indeed the problem, the fix might be as simple as a directory clear when running

python build_support/catalog/update.py --build --regenerate-images

?

Copy link
Copy Markdown
Member

@rahulsavani rahulsavani left a comment

Choose a reason for hiding this comment

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

Not working yet -- see my comment above.

@edwardchalstrey1
Copy link
Copy Markdown
Member Author

Unfortunately, this does not work yet.

My best guess is that the repo's live catalog/img directory contains everything from previous versions and is not getting cleared out. If that is indeed the problem, the fix might be as simple as a directory clear when running

python build_support/catalog/update.py --build --regenerate-images

?

I think ensuring when running the update script that the structure of catalog/img matches catalog is a check worth adding anyway, and actually should be done regardless of whether --build (for updating catalog.am) or --regenerate-images (for forcing the images to be remade for games which already have them as well as new ones) is present.

@edwardchalstrey1 edwardchalstrey1 changed the title Testing catalog build locally doesn't mistakenly retain old games (or require editable install) Testing catalog build locally doesn't mistakenly retain old games Jun 5, 2026
@edwardchalstrey1
Copy link
Copy Markdown
Member Author

@rahulsavani See the top comment - on further inspection, the 2 things that were broken and fixed by this PR are editable installs not picking up live changes properly and old images not being deleted. However, if you are doing a non editable pip install . each time before running the update script, this should already picking up changes correctly before this PR (see explanation above). So either way you shouldn't experience any more issues - let me know if you do!

Comment thread build_support/catalog/test_update.py Outdated
@rahulsavani
Copy link
Copy Markdown
Member

Unfortunately, Ed, this still doesn't seem to work for me. At this stage could you perhaps create a MWE of the problem locally so you can observe it for yourself and see if this fixes it for you?

@edwardchalstrey1
Copy link
Copy Markdown
Member Author

edwardchalstrey1 commented Jun 5, 2026

Unfortunately, Ed, this still doesn't seem to work for me. At this stage could you perhaps create a MWE of the problem locally so you can observe it for yourself and see if this fixes it for you?

What's the error you're getting? I've tested and not seeing any problems at this point

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: old catalog_data not cleared out on a pip install

2 participants