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

feat: Add plothist #27414

Merged
merged 1 commit into from
Aug 28, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions recipes/plothist/meta.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
{% set name = "plothist" %}
{% set version = "1.2.5" %}

package:
name: {{ name|lower }}
version: {{ version }}

source:
url: https://pypi.org/packages/source/{{ name[0] }}/{{ name }}/plothist-{{ version }}.tar.gz
sha256: b4606d9ff7db3cc1a7f6f5d5440fd959e1d1292818910cc0d142b23e65e4de76

build:
entry_points:
- install_latin_modern_fonts = plothist.scripts.install_latin_modern_fonts:install_latin_modern_fonts
noarch: python
Copy link
Member Author

Choose a reason for hiding this comment

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

Failing on Windows for noarch Python projects is allowed given what I've been told by conda-forge core people in the past so this is fine.

Copy link
Member

Choose a reason for hiding this comment

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

In this particular case the package won't be installable on Windows. If that is by design, we can merge this. However, it does seem like this package can be OS-noarch. See https://conda-forge.org/docs/maintainer/knowledge_base/#noarch-packages-with-os-specific-dependencies for more details.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ocefpaf In that case, given #27414 (comment) maybe we can wait for @0ctagon to make a patch release with cyrraz/plothist#176 merged and then we can have that patch release be the one that generates the feedstock (so we'd update the version and sha256).

Copy link
Member

Choose a reason for hiding this comment

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

B/c there is a patch already we can:

  • ignore that, merge it, and you'all fix it as soon as there is a new release
  • backport the patch now

I don't think it is worth making it OS-noarch b/c the fix is there already.

Copy link
Member Author

Choose a reason for hiding this comment

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

ignore that, merge it, and you'all fix it as soon as there is a new release

@ocefpaf I'm find with this suggestion. There should be a new patch release on PyPI before the end of the week so it would only be broken on Windows for a few days.

script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation
number: 0

requirements:
host:
- python >=3.7
- flit-core >=3.2,<4
- pip
run:
- python >=3.7
- boost-histogram >=1.4.0,<1.5.dev0
- numpy >=1.14.5
- matplotlib-base >=3.0
- pyyaml >=5.3.1
- scipy >=1.6.0
# Needed for install_latin_modern_fonts
- wget
- unzip
Comment on lines +31 to +33
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Should we wait for cyrraz/plothist#176 to be merged and a new small release? Or is it ok to be merged now?

Copy link
Member Author

@matthewfeickert matthewfeickert Aug 28, 2024

Choose a reason for hiding this comment

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

Conda-forge will create new release PRs automatically on the conda-forge/plothist-feedstock that will be created once this is merged, so no real need here to wait now. You can just remove those dependencies in the following conda-forge release PR.

Copy link
Member

@0ctagon 0ctagon Aug 29, 2024

Choose a reason for hiding this comment

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

I am not familiar with the workflow of conda-forge. We created the new release https://github.com/cyrraz/plothist/releases/tag/v1.2.6, but not PR got automatically created in https://github.com/conda-forge/plothist-feedstock. Should I do what the readme says (fork + PR) or did we miss a setup that would automatically create the PR?

Copy link
Member Author

@matthewfeickert matthewfeickert Aug 29, 2024

Choose a reason for hiding this comment

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

Once it is on PyPI a PR should get generated a few hours later. It takes some time for the checking scripts to loop through, but there will be a PR open within the next 24 hours.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you, it worked perfectly conda-forge/plothist-feedstock#1 😄


test:
imports:
- plothist
commands:
- pip check
# On macOS need /Users/<username>/Library/Fonts
- install_latin_modern_fonts # [linux]
requires:
- pip

about:
home: https://github.com/cyrraz/plothist
summary: Plot histograms in a scalable way and a beautiful style.
license: BSD-3-Clause
license_file: LICENSE
doc_url: https://plothist.readthedocs.io/
dev_url: https://github.com/cyrraz/plothist

extra:
recipe-maintainers:
- 0ctagon
Loading