Opened 9 years ago

Closed 9 years ago

#48413 closed submission (fixed)

py-horton @2.0.0 Quantum_chemistry_python_package

Reported by: tczorro (Derrick) Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc: petrrr, raimue (Rainer Müller)
Port: py-horton

Description (last modified by mf2k (Frank Schima))

Py-horton is a python package for quantum chemistry calculation developed by our group.

Main website: ​http://theochem.github.io/horton/

I submitted a same ticket but didn't get any reply. So I resubmit it again.

Previous link ticket:48360

Attachments (3)

Portfile (4.3 KB) - added by tczorro (Derrick) 9 years ago.
Portfile 1.1 (4.2 KB) - added by tczorro (Derrick) 9 years ago.
updated Portfile
Portfile 1.2 (2.1 KB) - added by tczorro (Derrick) 9 years ago.
upload 09/16/2015

Download all attachments as: .zip

Change History (22)

Changed 9 years ago by tczorro (Derrick)

Attachment: Portfile added

comment:1 Changed 9 years ago by mf2k (Frank Schima)

Port: py-horton added
Version: 2.3.3

Please do not submit duplicate submission tickets. If no one is responding it is because we are all busy. Ask on the Macports Developers mailing list instead.

comment:2 Changed 9 years ago by mf2k (Frank Schima)

Description: modified (diff)

comment:3 in reply to:  1 Changed 9 years ago by tczorro (Derrick)

Replying to mf2k@…:

Please do not submit duplicate submission tickets. If no one is responding it is because we are all busy. Ask on the Macports Developers mailing list instead.

I am sorry for submitting the same again. I sent a email but got this aotureplay:

You are not allowed to post to this mailing list, and your message has been automatically rejected. If you think that your messages are being rejected in error, contact the mailing list owner at macports-dev-owner@….

Sorry for duplicate tickets again.

Last edited 9 years ago by ryandesign (Ryan Carsten Schmidt) (previous) (diff)

comment:4 Changed 9 years ago by mf2k (Frank Schima)

Did you subscribe to the list? If so, did you email the list manager as suggested in the response?

comment:5 in reply to:  4 Changed 9 years ago by tczorro (Derrick)

Replying to mf2k@…:

Did you subscribe to the list? If so, did you email the list manager as suggested in the response?

I didn't subscribe to the list. I just subscribed it. Thank you for your help. I really appreciate it.

comment:6 Changed 9 years ago by petrrr

Cc: petr@… added

Cc Me!

comment:7 Changed 9 years ago by petrrr

Dear Reporter, you have now submitted five tickets for the same port submission: #48030, #48048, #48250, #48360, #48413. This is not really helpful, because relevant part of the discussion might get split across the tickets. Please indicate which of this ticket you consider the relevant one. Thanks!

comment:8 in reply to:  4 Changed 9 years ago by tczorro (Derrick)

Replying to mf2k@…:

Did you subscribe to the list? If so, did you email the list manager as suggested in the response?

Sorry for submitting too much. This is what I sent to the list and I think this one is the latest updated one.

comment:9 Changed 9 years ago by petrrr

Okay, I closed all other tickets.

But PLEASE never open a ticket on an already existing issue/topic (whatever it is submission, bug, etc.; own tickets of someone else) This just creates confusion and split discussions.

comment:10 in reply to:  9 Changed 9 years ago by tczorro (Derrick)

Replying to petr@…:

Okay, I closed all other tickets.

But PLEASE never open a ticket on an already existing issue/topic (whatever it is submission, bug, etc.; own tickets of someone else) This just creates confusion and split discussions.

Hello, would you please give my portfile a little bit comments or review.

comment:11 Changed 9 years ago by Russell-Jones-OxPhys (Russell Jones)

Hi Derrick,

Not a full review, but a few questions.

What happens if you try to build it with the OS X Accelerate framework instead of Atlas?

Does the Portfile build the things in https://github.com/theochem/horton/blob/master/depends/Makefile with clang?

It looks like it builds them as part of the port, which could be OK though is usually best avoided if possible. Did you check if those libraries are available as ports? Have you checked if they are linked against MacPorts libraries only (wiki:FAQ#ownlibs)?

My knowledge of Portfile development is somewhat second-hand, so I hope these are helpful.

By the way, which OS X version(s) are you testing on?

Russell

Last edited 9 years ago by ryandesign (Ryan Carsten Schmidt) (previous) (diff)

comment:12 in reply to:  11 Changed 9 years ago by tczorro (Derrick)

Replying to russell.jones@…:

Hi Derrick,

Not a full review, but a few questions.

What happens if you try to build it with the OS X Accelerate framework instead of Atlas?

Does the Portfile build the things in https://github.com/theochem/horton/blob/master/depends/Makefile with clang?

It looks like it builds them as part of the port, which could be OK though is usually best avoided if possible. Did you check if those libraries are available as ports? Have you checked if they are linked against MacPorts libraries only ( https://trac.macports.org/wiki/FAQ#ownlibs )?

My knowledge of Portfile development is somewhat second-hand, so I hope these are helpful.

By the way, which OS X version(s) are you testing on?

Russell

Hey Russell,

Thank you for your reviews.

Q1: What happens if you try to build it with the OS X Accelerate framework instead of Atlas? A: I thought it will go wrong. The thing is in the setup.py, it checks whether there is a blas and atlas library.

Q2: Does the Portfile build the things in A: Yes.

Q3: It looks like it builds them as part of the port, which could be OK though is usually best avoided if possible. Did you check if those libraries are available as ports? Have you checked if they are linked against MacPorts libraries only ( ​https://trac.macports.org/wiki/FAQ#ownlibs )? A: If you manually install this package you need to make libint and libxc yourself. But in this portfile, I port libint and libxc, both are available in macport. So it is fine. We do not need to build those parts

Q4: By the way, which OS X version(s) are you testing on?

A: I test on My own laptop. Both 10.10.3 and 10.10.4. The cfg files are for users to manually build Horton. In this portfile, I made a universal setting for all macport users. So I think it should be fine.

And I really appreciate your reviews. Derrick

Version 0, edited 9 years ago by tczorro (Derrick) (next)

comment:13 Changed 9 years ago by raimue (Rainer Müller)

As this port also provides scripts that are meant to be called by users, it would probably make more sense to name it simply horton. The py-* prefix is used for python modules that are only referenced as library. Also I recommend to put this port into the primary category science.

The ui_warn in the post-activate phase should be converted to port notes. It might get lost during installations this way, notes are all shown at the end of the install/update process of all ports. I think the intention was to only show this message if py27-numpy +atlas was not an active variant, but notes will be saved before the activate phase, so that needs to go into a pre-activate phase. Also, any markers to draw additional attention such as these asterisks are discouraged.

pre-activate {
    if {![active_variants py27-numpy atlas]} {
        notes {
            In order to have CHOLESKY work properly, ...
        }
    }
}

Also if py27-numpy was already installed, the given install command does not work. It requires an upgrade with the new variant, that would be: sudo port -n upgrade --enforce-variants py27-numpy +atlas.

comment:14 Changed 9 years ago by raimue (Rainer Müller)

Cc: raimue@… added

Cc Me!

Changed 9 years ago by tczorro (Derrick)

Attachment: Portfile 1.1 added

updated Portfile

comment:15 in reply to:  13 Changed 9 years ago by tczorro (Derrick)

Replying to raimue@…:

As this port also provides scripts that are meant to be called by users, it would probably make more sense to name it simply horton. The py-* prefix is used for python modules that are only referenced as library. Also I recommend to put this port into the primary category science.

The ui_warn in the post-activate phase should be converted to port notes. It might get lost during installations this way, notes are all shown at the end of the install/update process of all ports. I think the intention was to only show this message if py27-numpy +atlas was not an active variant, but notes will be saved before the activate phase, so that needs to go into a pre-activate phase. Also, any markers to draw additional attention such as these asterisks are discouraged.

pre-activate {
    if {![active_variants py27-numpy atlas]} {
        notes {
            In order to have CHOLESKY work properly, ...
        }
    }
}

Also if py27-numpy was already installed, the given install command does not work. It requires an upgrade with the new variant, that would be: sudo port -n upgrade --enforce-variants py27-numpy +atlas.

Thank you for your review. I changed the category to science, changed the ui_warn into notes. These changes do make the information looks better rather than the asterisks. Fot the name I am a little bit hesitate because our package is still a python module and right now only for python27. It works like a library, but it takes a little bit complicated steps to install. So I still prefer its name to be py27-horton so users can understand this is still a python package other than a individual software.

I upload the latest Portfile 1.1 in the attachment.

comment:16 Changed 9 years ago by petrrr

Hi Xiaotian,

here some notes on the updated Portfile:

I think above all you have not fully address Rainer's question on whether this should not be installed simply as an App, instead as module.

Here some background: The python portgroup supports to modes for modules and for applications, this is controlled by the name of the port (py-* are for modules, otherwise it is treated as application). Modules can be installed for multiple versions of python and conflicts are avoided by appending the -${python.branch} postfix.

In post-destroot you revert this behavior, making it impossible to install against several versions of python contemporary. As long as you do have only python 2.7, this will not surface, but then you probably do not intend to install a module and you should create an app.

If you think this should be a module and more Python versions could be supported in future, you need to remove the post-destroot overwrite, and if you want script names w/o suffix, you need to use the select mechanism.

I would recommend you to add openmaintainer.

The following settings are defaults and should be removed:

extract.suffix      .tar.gz

[...]
distname            ${name}-${version}
distfiles           ${distname}${extract.suffix}

Instead of file copy use the Macports alias copy

livecheck should be set correctly by the the Github portgroup, so avoid overwriting it.

Changed 9 years ago by tczorro (Derrick)

Attachment: Portfile 1.2 added

upload 09/16/2015

comment:17 in reply to:  16 ; Changed 9 years ago by tczorro (Derrick)

Replying to petr@…:

Hi Xiaotian,

here some notes on the updated Portfile:

I think above all you have not fully address Rainer's question on whether this should not be installed simply as an App, instead as module.

Here some background: The python portgroup supports to modes for modules and for applications, this is controlled by the name of the port (py-* are for modules, otherwise it is treated as application). Modules can be installed for multiple versions of python and conflicts are avoided by appending the -${python.branch} postfix.

In post-destroot you revert this behavior, making it impossible to install against several versions of python contemporary. As long as you do have only python 2.7, this will not surface, but then you probably do not intend to install a module and you should create an app.

If you think this should be a module and more Python versions could be supported in future, you need to remove the post-destroot overwrite, and if you want script names w/o suffix, you need to use the select mechanism.

I would recommend you to add openmaintainer.

The following settings are defaults and should be removed:

extract.suffix      .tar.gz

[...]
distname            ${name}-${version}
distfiles           ${distname}${extract.suffix}

Instead of file copy use the Macports alias copy

livecheck should be set correctly by the the Github portgroup, so avoid overwriting it.

Thanks for your review. The latest 1.2 portfile was just uploaded. I changed it into an application and it make the portfile much easier and readable. Duplicated parts are deleted as well as the some of the redundent post-destroot part.

livecheck.type is changed to none

Test it on my own laptop. Installed smoothly and all the tests are ok.

Would you please take some time to review it again.

BTW, I would like to add openmaintainer but how do I do that.

Last edited 9 years ago by tczorro (Derrick) (previous) (diff)

comment:18 in reply to:  17 Changed 9 years ago by mf2k (Frank Schima)

Replying to yangx59@…:

BTW, I would like to add openmaintainer but how do I do that.

Simply change your maintainers line to the following:

maintainers         mcmaster.ca:yangx59 openmaintainer

comment:19 in reply to:  17 Changed 9 years ago by raimue (Rainer Müller)

Resolution: fixed
Status: newclosed

Replying to yangx59@…:

livecheck.type is changed to none

This disables livecheck completely. What petr@ meant was to remove this line as the github portgroup already defines a working livecheck.

Test it on my own laptop. Installed smoothly and all the tests are ok.

Would you please take some time to review it again.

BTW, I would like to add openmaintainer but how do I do that.

I committed the port in r140342 with the following small changes:

  • removed livecheck.type
  • add openmaintainer
  • use science as primary category

Thank you for the submission! If you as maintainer of the port want to update it, please open a new ticket with a patch and always add maintainer haspatch to the keywords of the ticket.

Note: See TracTickets for help on using tickets.