otb: init at 9.0.0 #325630

daspk04 · 2024-07-08T19:45:04Z

Description of changes

Things done

Add a 👍 reaction to pull requests you find important.

pkgs/by-name/it/itk_4_13/package.nix

pkgs/by-name/ot/otb/package.nix

pkgs/by-name/sh/shark/package.nix

pkgs/by-name/ot/otb/package.nix

SuperSandro2000 · 2024-07-12T09:10:55Z

pkgs/by-name/ot/otb/package.nix

+    ];
+
+
+    propagatedBuildInputs = [


Why do we want to propagate all of this?

Thanks @SuperSandro2000.

As OTB allows to build (remote) modules (officially not part of) on top of it, so we propagate the inputs to able to build remote modules based on OTB via nix as well. As I wasn't sure if all are needed so I tried to follow similar to ITK c.f

Example of remote modules for OTB:

https://github.com/remicres/otbtf

https://github.com/remicres/otb-prefetch

https://gitlab.orfeo-toolbox.org/orfeotoolbox/otb/-/tree/develop/Modules/Remote?ref_type=heads

Not sure how this will integrate with nix. First of all propagatedBuildInputs are merged into buildInputs, so we don't need anything in buildInputs that is already in propagatedBuildInputs.

The other things, we don't want to propagate things if possible. Are those modules build with nix? If so we should move things to passthru. If not, then we just keep this ugly workaround for now until we have something better.

Thanks @SuperSandro2000 !

Currently those modules are not built with nix.

pkgs/by-name/ot/otb/package.nix

pkgs/by-name/it/itk_4_13/package.nix

pkgs/by-name/ot/otb/package.nix

daspk04 · 2024-07-13T18:58:02Z

@jackyliu16 I did some more changes. Will it be possible if you could review one more time ?

jackyliu16 · 2024-07-17T16:18:14Z

@daspk04 To be honst, I not a specialist in packing.
I can't comment much on the more detailed parts of your submission, which I not familiar with these things.

But the things I could suggest is follow:

You should try to use nixfmt-rfc-style to format your code.
maybe you could run something like nix run .#nixfmt-rfc-style -- pkgs/by-name/otb/<specify file> to formatting your file base on [RFC 0166] Nix formatting, take two rfcs#166
You may consider to use override to cover some Attr in pkgs/development/libraries/itk , rather than recreate a package . (maybe you could use it's generate.nix file ?, I not sure)

daspk04 · 2024-07-21T15:19:30Z

@daspk04 To be honst, I not a specialist in packing. I can't comment much on the more detailed parts of your submission, which I not familiar with these things.

But the things I could suggest is follow:

You should try to use nixfmt-rfc-style to format your code.
maybe you could run something like nix run .#nixfmt-rfc-style -- pkgs/by-name/otb/<specify file> to formatting your file base on [RFC 0166] Nix formatting, take two rfcs#166

Thanks @jackyliu16. I have formatted using the nixfmt-rfc-style.

You may consider to use override to cover some Attr in pkgs/development/libraries/itk , rather than recreate a package . (maybe you could use it's generate.nix file ?, I not sure)

As suggested by @SuperSandro2000 c.f, either override or local to the package I followed the second approach as it seemed easier and more logical to me, possibly can be done using overwrite but I'm still not sure how to do it for now😅

daspk04 · 2024-07-21T15:29:08Z

Hi @SuperSandro2000 ,

Will it possible if you could review it one more time ?

I have formatted using nixfmt-rfc-style as suggested c.f. There are few question (c.f, c.f ) if you could suggested what to be done then that would be helpful and I can try to make changes accordingly. 🙏

daspk04 · 2024-08-28T01:40:32Z

Hi @SuperSandro2000 ,

I would like this to be merged so I would like to have your suggestion regarding the questions that I have asked earlier in the comments, if you could please suggest what changes are further needed for this to be merged.

pkgs/by-name/ot/otb/package.nix

SuperSandro2000 · 2024-08-28T09:41:53Z

pkgs/by-name/ot/otb/package.nix

+    optionals enablePython (with python3.pkgs; [ numpy ]) ++ (extraPythonPackages python3.pkgs);
+
+  otb-itk = callPackage ./itk_4_13/package.nix { };
+  otb-shark = shark.override { enableOpenMP = enableOpenMP; };


Why not enable openmp by default and drop the option?

Thanks @SuperSandro2000 !

By default it is OFF c.f so I have kept it as it is.

pkgs/by-name/ot/otb/package.nix

SuperSandro2000 · 2024-08-28T09:42:15Z

pkgs/by-name/ot/otb/package.nix

+  enableFeatureExtraction ? true,
+  enableHyperspectral ? true,
+  enableLearning ? true,
+  enableMiscellaneous ? true,
+  enableOpenMP ? false,
+  enablePython ? true,
+  extraPythonPackages ? ps: with ps; [ ],
+  enableRemote ? true,
+  enableSAR ? true,
+  enableSegmentation ? true,
+  enableStereoProcessing ? true,


Do we really need those options? or can we just turn them on by default and make things simple?

Actually by default OTB has only core modules and all these options are OFF. They have mentioned in doc that incase one needs some module then one can turn on, but then that requires compiling again from source. I would prefer to keep these option as this is similar to as mentioned in doc, here I kept all ON by default (opposite from OTB) as compiling all takes a bit of time (~30min) incase one doesn't need some some module they can override.

Ideally I would prefer something like this in Nixpkgs so that one don't have to build or compile (possibly can be done later):

otb (full)

otbMinimal (only core)

otbSAR (otbMinimal + SAR related modules)

otbLearning (otbMinimal + Learning related modules)

...

SuperSandro2000 · 2024-08-28T09:43:02Z

2. You may consider to use override to cover some Attr in pkgs/development/libraries/itk , rather than recreate a package . (maybe you could use it's generate.nix file ?, I not sure)

That would be nice but the files are already pretty different and maintaining backwards compatibility might be a pain in the future.

daspk04 · 2024-08-29T11:09:44Z

Hi @SuperSandro2000 !

I have applied all the suggested changes.

daspk04 · 2024-10-05T04:46:08Z

Hi @SuperSandro2000 !

Will it possible if you could review once more ? I have already applied all of your suggested changes.

FliegendeWurst

LGTM mostly

pkgs/by-name/sh/shark/package.nix

pkgs/by-name/ot/otb/package.nix

daspk04 · 2024-11-14T15:37:31Z

Thanks @FliegendeWurst for the review!

I have applied your suggested changes.

pkgs/by-name/ot/otb/itk_4_13/package.nix

* an old version of itk with extra configuration required for OTB 9.0.0

* init at an unstable version based on commit on 2024-05-25 as no more official release from 4.0.1 onwards.

pkgs/by-name/ot/otb/itk_4_13/package.nix

daspk04 · 2024-12-02T16:45:10Z

Hi @SuperSandro2000 @FliegendeWurst,

I have addressed all the comments. I don't have any further changes. Will it it be possible to merge this ?

GaetanLepage · 2024-12-02T17:53:38Z

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 325630

`x86_64-linux`

✅ 2 packages built:

otb
shark

`aarch64-linux`

✅ 2 packages built:

otb
shark

`x86_64-darwin`

⏩ 1 package marked as broken and skipped:

otb

✅ 1 package built:

shark

`aarch64-darwin`

⏩ 1 package marked as broken and skipped:

otb

✅ 1 package built:

shark

maintainers: add @daspk04

3ea89d7

github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Jul 8, 2024

NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jul 8, 2024

daspk04 force-pushed the add-otb-9.0.0 branch from 16e8ee7 to 0f8d329 Compare July 8, 2024 20:04

daspk04 mentioned this pull request Jul 8, 2024

New package: add orfeo toolbox (otb) package imincik/geospatial-nix#503

Open

jackyliu16 reviewed Jul 8, 2024

View reviewed changes

pkgs/by-name/it/itk_4_13/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/ot/otb/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/ot/otb/package.nix Outdated Show resolved Hide resolved

daspk04 force-pushed the add-otb-9.0.0 branch from 0f8d329 to 605b356 Compare July 9, 2024 23:12

ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jul 10, 2024

jackyliu16 requested a review from SuperSandro2000 July 10, 2024 05:07

SuperSandro2000 reviewed Jul 12, 2024

View reviewed changes

daspk04 force-pushed the add-otb-9.0.0 branch 2 times, most recently from e83a4d6 to dbdaf48 Compare July 13, 2024 17:39

daspk04 commented Jul 13, 2024

View reviewed changes

pkgs/by-name/ot/otb/package.nix Show resolved Hide resolved

daspk04 commented Jul 13, 2024

View reviewed changes

pkgs/by-name/ot/otb/package.nix Outdated Show resolved Hide resolved

daspk04 commented Jul 13, 2024

View reviewed changes

pkgs/by-name/ot/otb/package.nix Outdated Show resolved Hide resolved

daspk04 requested review from SuperSandro2000 and jackyliu16 July 13, 2024 18:41

daspk04 force-pushed the add-otb-9.0.0 branch from 3462b7c to b1e8565 Compare July 21, 2024 15:06

SuperSandro2000 reviewed Aug 28, 2024

View reviewed changes

daspk04 force-pushed the add-otb-9.0.0 branch 2 times, most recently from cf95954 to 4243f9d Compare August 29, 2024 10:41

daspk04 requested a review from SuperSandro2000 October 5, 2024 04:46

FliegendeWurst reviewed Nov 3, 2024

View reviewed changes

pkgs/by-name/sh/shark/package.nix Outdated Show resolved Hide resolved

pkgs/by-name/ot/otb/package.nix Outdated Show resolved Hide resolved

daspk04 force-pushed the add-otb-9.0.0 branch from 4243f9d to 7263a69 Compare November 14, 2024 15:33

daspk04 requested a review from FliegendeWurst November 14, 2024 15:44

ofborg bot added 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Nov 15, 2024

FliegendeWurst reviewed Nov 16, 2024

View reviewed changes

pkgs/by-name/ot/otb/itk_4_13/package.nix Show resolved Hide resolved

daspk04 added 3 commits November 17, 2024 12:24

itk_4_13: init at 4.13.3

b4fadb9

* an old version of itk with extra configuration required for OTB 9.0.0

shark: init at 4.0-unstable-2024-05-25

1d36631

* init at an unstable version based on commit on 2024-05-25 as no more official release from 4.0.1 onwards.

otb: init at 9.0.0

5f047b8

daspk04 force-pushed the add-otb-9.0.0 branch from 7263a69 to 5f047b8 Compare November 17, 2024 05:27

FliegendeWurst approved these changes Nov 18, 2024

View reviewed changes

pkgs/by-name/ot/otb/itk_4_13/package.nix Show resolved Hide resolved

wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 19, 2024

FliegendeWurst added the needs_merger (old Marvin label, do not use) label Dec 2, 2024

symphorien merged commit d1ed012 into NixOS:master Dec 2, 2024
31 checks passed

daspk04 deleted the add-otb-9.0.0 branch December 3, 2024 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

otb: init at 9.0.0 #325630

otb: init at 9.0.0 #325630

daspk04 commented Jul 8, 2024

SuperSandro2000 Jul 12, 2024

daspk04 Jul 13, 2024

SuperSandro2000 Aug 28, 2024

daspk04 Aug 29, 2024

daspk04 commented Jul 13, 2024

jackyliu16 commented Jul 17, 2024

daspk04 commented Jul 21, 2024 •

edited

Loading

daspk04 commented Jul 21, 2024

daspk04 commented Aug 28, 2024

SuperSandro2000 Aug 28, 2024

daspk04 Aug 29, 2024

SuperSandro2000 Aug 28, 2024

daspk04 Aug 29, 2024 •

edited

Loading

SuperSandro2000 commented Aug 28, 2024

daspk04 commented Aug 29, 2024

daspk04 commented Oct 5, 2024

FliegendeWurst left a comment

daspk04 commented Nov 14, 2024

daspk04 commented Dec 2, 2024 •

edited

Loading

GaetanLepage commented Dec 2, 2024

otb: init at 9.0.0 #325630

otb: init at 9.0.0 #325630

Conversation

daspk04 commented Jul 8, 2024

Description of changes

Things done

SuperSandro2000 Jul 12, 2024

Choose a reason for hiding this comment

daspk04 Jul 13, 2024

Choose a reason for hiding this comment

SuperSandro2000 Aug 28, 2024

Choose a reason for hiding this comment

daspk04 Aug 29, 2024

Choose a reason for hiding this comment

daspk04 commented Jul 13, 2024

jackyliu16 commented Jul 17, 2024

daspk04 commented Jul 21, 2024 • edited Loading

daspk04 commented Jul 21, 2024

daspk04 commented Aug 28, 2024

SuperSandro2000 Aug 28, 2024

Choose a reason for hiding this comment

daspk04 Aug 29, 2024

Choose a reason for hiding this comment

SuperSandro2000 Aug 28, 2024

Choose a reason for hiding this comment

daspk04 Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

SuperSandro2000 commented Aug 28, 2024

daspk04 commented Aug 29, 2024

daspk04 commented Oct 5, 2024

FliegendeWurst left a comment

Choose a reason for hiding this comment

daspk04 commented Nov 14, 2024

daspk04 commented Dec 2, 2024 • edited Loading

GaetanLepage commented Dec 2, 2024

nixpkgs-review result

x86_64-linux

aarch64-linux

x86_64-darwin

aarch64-darwin

daspk04 commented Jul 21, 2024 •

edited

Loading

daspk04 Aug 29, 2024 •

edited

Loading

daspk04 commented Dec 2, 2024 •

edited

Loading

`nixpkgs-review` result

`x86_64-linux`

`aarch64-linux`

`x86_64-darwin`

`aarch64-darwin`