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

Expose Fit Internals #217

Open
curran opened this issue Sep 12, 2020 · 7 comments · May be fixed by #220
Open

Expose Fit Internals #217

curran opened this issue Sep 12, 2020 · 7 comments · May be fixed by #220

Comments

@curran
Copy link
Contributor

curran commented Sep 12, 2020

Faced with the task of adding fit methods to geo-albers-usa-territories , I'm a bit stuck.

Ideally I could follow the pattern found in albersUsa.js, namely

import {fitExtent, fitSize, fitWidth, fitHeight} from "./fit.js";

However, since the projection is not inside this repository, and those internals from fit.js are not exported from the top level of this package, I see no way to access them. Am I missing an approach that would work?

FWIW I did try

import { fitExtent, fitSize, fitWidth, fitHeight } from 'd3-geo/src/fit.js';

But that feels janky, and caused some headaches with Rollup (here's what I tried, for reference).

I propose to export these methods from the d3-geo package, for use by projections defined outside this project.

Thoughts?

@curran
Copy link
Contributor Author

curran commented Sep 12, 2020

That might look something like adding the following to index.js:

(long names as this is probably a rare use case, and because these will end up on d3 itself)

export {
  fitExtent as geoProjectionFitExtent,
  fitSize as geoProjectionFitSize,
  fitWidth as geoProjectionFitWidth,
  fitHeight as geoProjectionFitHeight
} from "./projection/fit.js";

Then in the externally defined projection I could say:

import {
  geoProjectionFitExtent as fitExtent,
  geoProjectionFitSize as fitSize,
  geoProjectionFitWidth as fitWidth,
  geoProjectionFitHeight as fitHeight
} from 'd3-geo';

@Fil
Copy link
Member

Fil commented Sep 12, 2020

An alternative approach would be to finally offer a composite projections construct, that would deal with all the peculiarities (clipping, inverse, etc).

@curran
Copy link
Contributor Author

curran commented Sep 12, 2020

Interesting idea! Were there already discussions along the lines of a composite projections construct?

More sleuthing reveals another approach, though not sure if this is applicable https://github.com/d3/d3-geo-projection/blob/master/src/gilbert.js#L37

Maybe I could just say

  property("fitExtent");
  property("fitHeight");
  property("fitSize");
  property("fitWidth");

@curran
Copy link
Contributor Author

curran commented Oct 14, 2020

Considering submitting a PR with the change to expose these.

curran added a commit to curran/d3-geo that referenced this issue Oct 28, 2020
@curran curran linked a pull request Oct 28, 2020 that will close this issue
@Fil
Copy link
Member

Fil commented May 31, 2021

It looks like @HarryStevens has an alternative here https://observablehq.com/@washpostgraphics/geo-albers-usa-pr

@HarryStevens
Copy link

It looks like @HarryStevens has an alternative here https://observablehq.com/@washpostgraphics/geo-albers-usa-pr

Yep, I just kept copy-pasting code from d3-geo until it stopped throwing errors.

@nthitz
Copy link

nthitz commented Sep 2, 2021

apologies slightly off topic, but any chance of publishing https://observablehq.com/@washpostgraphics/geo-albers-usa-pr to npm for easier use off of observable?

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

Successfully merging a pull request may close this issue.

4 participants